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

Proposed by Thomas Ward on 2020-05-08
Status: Merged
Approved by: Colin Watson on 2020-05-11
Approved revision: 3a2f6f99c56d9f026a4775ae71e4d3e9985b966a
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~teward/launchpad:dmarc-for-bugs
Merge into: launchpad:master
Diff against target: 170 lines (+30/-61)
3 files modified
lib/lp/bugs/doc/bugnotification-email.txt (+20/-25)
lib/lp/bugs/doc/bugnotification-sending.txt (+4/-4)
lib/lp/bugs/mail/bugnotificationbuilder.py (+6/-32)
Reviewer Review Type Date Requested Status
Colin Watson 2020-05-08 Approve on 2020-05-11
Review via email: mp+383643@code.launchpad.net

Commit message

Use DMARC compliant From addresses in bug notifications

Description of the change

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.

To post a comment you must log in.
Andrew Johnson (anj) wrote :

+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) wrote :

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) wrote :

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

review: Approve
~teward/launchpad:dmarc-for-bugs updated on 2020-05-11
3a2f6f9... by Thomas Ward on 2020-05-11

Adjust changes per Colin Watson suggestions/requests.

Thomas Ward (teward) wrote :

> 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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:
454 >>> get_bugmail_replyto_address(bug_four)454 >>> get_bugmail_replyto_address(bug_four)
455 u'Bug 4 <4@bugs.launchpad.net>'455 u'Bug 4 <4@bugs.launchpad.net>'
456456
457The From address generator handles a few special cases. The trivial case457In order to send DMARC-compliant bug notifications, the From address generator
458is, well, trivial. Stuart has four email addresses:458is also quite straightforward and uses the bug's email address for the From
459address, while adjusting the friendly display name field.
460
461This applies for all users. For example, Stuart has four email addresses:
459462
460 >>> stub = getUtility(IPersonSet).getByName("stub")463 >>> stub = getUtility(IPersonSet).getByName("stub")
461 >>> [(email.email, email.status.name) for email464 >>> [(email.email, email.status.name) for email
@@ -465,47 +468,39 @@ is, well, trivial. Stuart has four email addresses:
465 (u'stub@fastmail.fm', 'NEW'),468 (u'stub@fastmail.fm', 'NEW'),
466 (u'zen@shangri-la.dropbear.id.au', 'OLD')]469 (u'zen@shangri-la.dropbear.id.au', 'OLD')]
467470
468But we use his preferred one:471However, because of DMARC compliance, we only use the bug's email address in the
472From field, with Stuart's name in the 'display name' portion of the email address:
469473
470 >>> get_bugmail_from_address(stub, bug_four)474 >>> get_bugmail_from_address(stub, bug_four)
471 'Stuart Bishop <stuart.bishop@canonical.com>'475 'Stuart Bishop <4@bugs.launchpad.net>'
472476
473Now, mpo doesn't have a validated email address, but we pick out the477This also happens for users with hidden addresses:
474first address we find:478
479 >>> private_person = factory.makePerson(
480 ... email="hidden@example.com", displayname="Ford Prefect")
481 >>> private_person.hide_email_addresses = True
482 >>> get_bugmail_from_address(private_person, bug_four)
483 'Ford Prefect <4@bugs.launchpad.net>'
484
485It also behaves the same for users with no verified email addresses:
475486
476 >>> mpo = getUtility(IPersonSet).getByName("mpo")487 >>> mpo = getUtility(IPersonSet).getByName("mpo")
477 >>> get_bugmail_from_address(mpo, bug_four)488 >>> get_bugmail_from_address(mpo, bug_four)
478 '=?utf-8?b?TWF0dGkgUMO2bGzDpA==?= <mpo@iki.fi>'489 '=?utf-8?b?TWF0dGkgUMO2bGzDpA==?= <4@bugs.launchpad.net>'
479
480(As you can see in the above example, get_bugmail_from_address() takes
481care of encoding the person's displayname correctly.)
482490
483The team janitor doesn't have an email address at all!491This also happens for the team janitor:
484492
485 >>> janitor = getUtility(IPersonSet).getByName("team-membership-janitor")493 >>> janitor = getUtility(IPersonSet).getByName("team-membership-janitor")
486 >>> get_bugmail_from_address(janitor, bug_four)494 >>> get_bugmail_from_address(janitor, bug_four)
487 'Team Membership Janitor <4@bugs.launchpad.net>'495 'Team Membership Janitor <4@bugs.launchpad.net>'
488496
489The Launchpad Janitor celebrity isn't a real user, and shouldn't be497And it also applies for the Launchpad Janitor:
490sending mail. Notifications from the janitor are sent with the address
491of the bug itself.
492498
493 >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities499 >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
494 >>> lp_janitor = getUtility(ILaunchpadCelebrities).janitor500 >>> lp_janitor = getUtility(ILaunchpadCelebrities).janitor
495 >>> get_bugmail_from_address(lp_janitor, bug_four)501 >>> get_bugmail_from_address(lp_janitor, bug_four)
496 'Launchpad Bug Tracker <4@bugs.launchpad.net>'502 'Launchpad Bug Tracker <4@bugs.launchpad.net>'
497503
498If a person has specified that their email remain private,
499get_bugmail_from_address() will return their display name with bug's
500email address.
501
502 >>> private_person = factory.makePerson(
503 ... email="hidden@example.com", displayname="Ford Prefect")
504 >>> private_person.hide_email_addresses = True
505 >>> get_bugmail_from_address(private_person, bug_four)
506 'Ford Prefect <4@bugs.launchpad.net>'
507
508
509Construction of bug notification emails504Construction of bug notification emails
510---------------------------------------505---------------------------------------
511506
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:
947947
948 >>> print_notification(collated_messages['concise@example.com'][0])948 >>> print_notification(collated_messages['concise@example.com'][0])
949 To: concise@example.com949 To: concise@example.com
950 From: Verbose Person <verbose@example.com>950 From: Verbose Person <...@bugs.launchpad.net>
951 Subject: [Bug ...] subject951 Subject: [Bug ...] subject
952 X-Launchpad-Message-Rationale: Subscriber952 X-Launchpad-Message-Rationale: Subscriber
953 X-Launchpad-Message-For: concise-person953 X-Launchpad-Message-For: concise-person
@@ -975,7 +975,7 @@ that gets verbose email.
975975
976 >>> print_notification(collated_messages['verboseteam@example.com'][0])976 >>> print_notification(collated_messages['verboseteam@example.com'][0])
977 To: verboseteam@example.com977 To: verboseteam@example.com
978 From: Verbose Person <verbose@example.com>978 From: Verbose Person <...@bugs.launchpad.net>
979 Subject: [Bug ...] subject979 Subject: [Bug ...] subject
980 X-Launchpad-Message-Rationale: Subscriber @verboseteam980 X-Launchpad-Message-Rationale: Subscriber @verboseteam
981 X-Launchpad-Message-For: verboseteam981 X-Launchpad-Message-For: verboseteam
@@ -995,7 +995,7 @@ Whereas Verbose Person does get the description and task status:
995995
996 >>> print_notification(collated_messages['verbose@example.com'][0])996 >>> print_notification(collated_messages['verbose@example.com'][0])
997 To: verbose@example.com997 To: verbose@example.com
998 From: Verbose Person <verbose@example.com>998 From: Verbose Person <...@bugs.launchpad.net>
999 Subject: [Bug ...] subject999 Subject: [Bug ...] subject
1000 X-Launchpad-Message-Rationale: Subscriber1000 X-Launchpad-Message-Rationale: Subscriber
1001 X-Launchpad-Message-For: verbose-person1001 X-Launchpad-Message-For: verbose-person
@@ -1026,7 +1026,7 @@ And Concise Team Person does too, even though their team doesn't want them:
10261026
1027 >>> print_notification(collated_messages['conciseteam@example.com'][0])1027 >>> print_notification(collated_messages['conciseteam@example.com'][0])
1028 To: conciseteam@example.com1028 To: conciseteam@example.com
1029 From: Verbose Person <verbose@example.com>1029 From: Verbose Person <...@bugs.launchpad.net>
1030 Subject: [Bug ...] subject1030 Subject: [Bug ...] subject
1031 X-Launchpad-Message-Rationale: Subscriber @conciseteam1031 X-Launchpad-Message-Rationale: Subscriber @conciseteam
1032 X-Launchpad-Message-For: conciseteam1032 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):
43def get_bugmail_from_address(person, bug):43def get_bugmail_from_address(person, bug):
44 """Returns the right From: address to use for a bug notification."""44 """Returns the right From: address to use for a bug notification."""
45 if person == getUtility(ILaunchpadCelebrities).janitor:45 if person == getUtility(ILaunchpadCelebrities).janitor:
46 return format_address(46 displayname = 'Launchpad Bug Tracker'
47 'Launchpad Bug Tracker',47 else:
48 "%s@%s" % (bug.id, config.launchpad.bugs_domain))48 displayname = person.display_name
4949
50 if person.hide_email_addresses:50 return format_address(displayname,
51 return format_address(51 "%s@%s" % (bug.id, config.launchpad.bugs_domain))
52 person.displayname,
53 "%s@%s" % (bug.id, config.launchpad.bugs_domain))
54
55 if person.preferredemail is not None:
56 return format_address(person.displayname, person.preferredemail.email)
57
58 # XXX: Bjorn Tillenius 2006-04-05:
59 # The person doesn't have a preferred email set, but they
60 # added a comment (either via the email UI, or because they were
61 # imported as a deaf reporter). It shouldn't be possible to use the
62 # email UI if you don't have a preferred email set, but work around
63 # it for now by trying hard to find the right email address to use.
64 email_addresses = shortlist(
65 getUtility(IEmailAddressSet).getByPerson(person))
66 if not email_addresses:
67 # XXX: Bjorn Tillenius 2006-05-21 bug=33427:
68 # A user should always have at least one email address,
69 # but due to bug #33427, this isn't always the case.
70 return format_address(person.displayname,
71 "%s@%s" % (bug.id, config.launchpad.bugs_domain))
72
73 # At this point we have no validated emails to use: if any of the
74 # person's emails had been validated the preferredemail would be
75 # set. Since we have no idea of which email address is best to use,
76 # we choose the first one.
77 return format_address(person.displayname, email_addresses[0].email)
7852
7953
80def get_bugmail_replyto_address(bug):54def get_bugmail_replyto_address(bug):

Subscribers

People subscribed via source and target branches