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
1diff --git a/lib/lp/bugs/doc/bugnotification-email.txt b/lib/lp/bugs/doc/bugnotification-email.txt
2index d9c5fbc..38c599b 100644
3--- a/lib/lp/bugs/doc/bugnotification-email.txt
4+++ b/lib/lp/bugs/doc/bugnotification-email.txt
5@@ -454,8 +454,11 @@ The Reply-To address generation is straightforward:
6 >>> get_bugmail_replyto_address(bug_four)
7 u'Bug 4 <4@bugs.launchpad.net>'
8
9-The From address generator handles a few special cases. The trivial case
10-is, well, trivial. Stuart has four email addresses:
11+In order to send DMARC-compliant bug notifications, the From address generator
12+is also quite straightforward and uses the bug's email address for the From
13+address, while adjusting the friendly display name field.
14+
15+This applies for all users. For example, Stuart has four email addresses:
16
17 >>> stub = getUtility(IPersonSet).getByName("stub")
18 >>> [(email.email, email.status.name) for email
19@@ -465,47 +468,39 @@ is, well, trivial. Stuart has four email addresses:
20 (u'stub@fastmail.fm', 'NEW'),
21 (u'zen@shangri-la.dropbear.id.au', 'OLD')]
22
23-But we use his preferred one:
24+However, because of DMARC compliance, we only use the bug's email address in the
25+From field, with Stuart's name in the 'display name' portion of the email address:
26
27 >>> get_bugmail_from_address(stub, bug_four)
28- 'Stuart Bishop <stuart.bishop@canonical.com>'
29+ 'Stuart Bishop <4@bugs.launchpad.net>'
30
31-Now, mpo doesn't have a validated email address, but we pick out the
32-first address we find:
33+This also happens for users with hidden addresses:
34+
35+ >>> private_person = factory.makePerson(
36+ ... email="hidden@example.com", displayname="Ford Prefect")
37+ >>> private_person.hide_email_addresses = True
38+ >>> get_bugmail_from_address(private_person, bug_four)
39+ 'Ford Prefect <4@bugs.launchpad.net>'
40+
41+It also behaves the same for users with no verified email addresses:
42
43 >>> mpo = getUtility(IPersonSet).getByName("mpo")
44 >>> get_bugmail_from_address(mpo, bug_four)
45- '=?utf-8?b?TWF0dGkgUMO2bGzDpA==?= <mpo@iki.fi>'
46-
47-(As you can see in the above example, get_bugmail_from_address() takes
48-care of encoding the person's displayname correctly.)
49+ '=?utf-8?b?TWF0dGkgUMO2bGzDpA==?= <4@bugs.launchpad.net>'
50
51-The team janitor doesn't have an email address at all!
52+This also happens for the team janitor:
53
54 >>> janitor = getUtility(IPersonSet).getByName("team-membership-janitor")
55 >>> get_bugmail_from_address(janitor, bug_four)
56 'Team Membership Janitor <4@bugs.launchpad.net>'
57
58-The Launchpad Janitor celebrity isn't a real user, and shouldn't be
59-sending mail. Notifications from the janitor are sent with the address
60-of the bug itself.
61+And it also applies for the Launchpad Janitor:
62
63 >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
64 >>> lp_janitor = getUtility(ILaunchpadCelebrities).janitor
65 >>> get_bugmail_from_address(lp_janitor, bug_four)
66 'Launchpad Bug Tracker <4@bugs.launchpad.net>'
67
68-If a person has specified that their email remain private,
69-get_bugmail_from_address() will return their display name with bug's
70-email address.
71-
72- >>> private_person = factory.makePerson(
73- ... email="hidden@example.com", displayname="Ford Prefect")
74- >>> private_person.hide_email_addresses = True
75- >>> get_bugmail_from_address(private_person, bug_four)
76- 'Ford Prefect <4@bugs.launchpad.net>'
77-
78-
79 Construction of bug notification emails
80 ---------------------------------------
81
82diff --git a/lib/lp/bugs/doc/bugnotification-sending.txt b/lib/lp/bugs/doc/bugnotification-sending.txt
83index d8dc14f..19cd3a8 100644
84--- a/lib/lp/bugs/doc/bugnotification-sending.txt
85+++ b/lib/lp/bugs/doc/bugnotification-sending.txt
86@@ -947,7 +947,7 @@ We can see that Concise Person doesn't receive verbose notifications:
87
88 >>> print_notification(collated_messages['concise@example.com'][0])
89 To: concise@example.com
90- From: Verbose Person <verbose@example.com>
91+ From: Verbose Person <...@bugs.launchpad.net>
92 Subject: [Bug ...] subject
93 X-Launchpad-Message-Rationale: Subscriber
94 X-Launchpad-Message-For: concise-person
95@@ -975,7 +975,7 @@ that gets verbose email.
96
97 >>> print_notification(collated_messages['verboseteam@example.com'][0])
98 To: verboseteam@example.com
99- From: Verbose Person <verbose@example.com>
100+ From: Verbose Person <...@bugs.launchpad.net>
101 Subject: [Bug ...] subject
102 X-Launchpad-Message-Rationale: Subscriber @verboseteam
103 X-Launchpad-Message-For: verboseteam
104@@ -995,7 +995,7 @@ Whereas Verbose Person does get the description and task status:
105
106 >>> print_notification(collated_messages['verbose@example.com'][0])
107 To: verbose@example.com
108- From: Verbose Person <verbose@example.com>
109+ From: Verbose Person <...@bugs.launchpad.net>
110 Subject: [Bug ...] subject
111 X-Launchpad-Message-Rationale: Subscriber
112 X-Launchpad-Message-For: verbose-person
113@@ -1026,7 +1026,7 @@ And Concise Team Person does too, even though their team doesn't want them:
114
115 >>> print_notification(collated_messages['conciseteam@example.com'][0])
116 To: conciseteam@example.com
117- From: Verbose Person <verbose@example.com>
118+ From: Verbose Person <...@bugs.launchpad.net>
119 Subject: [Bug ...] subject
120 X-Launchpad-Message-Rationale: Subscriber @conciseteam
121 X-Launchpad-Message-For: conciseteam
122diff --git a/lib/lp/bugs/mail/bugnotificationbuilder.py b/lib/lp/bugs/mail/bugnotificationbuilder.py
123index f784d30..0846bb7 100644
124--- a/lib/lp/bugs/mail/bugnotificationbuilder.py
125+++ b/lib/lp/bugs/mail/bugnotificationbuilder.py
126@@ -43,38 +43,12 @@ def format_rfc2822_date(date):
127 def get_bugmail_from_address(person, bug):
128 """Returns the right From: address to use for a bug notification."""
129 if person == getUtility(ILaunchpadCelebrities).janitor:
130- return format_address(
131- 'Launchpad Bug Tracker',
132- "%s@%s" % (bug.id, config.launchpad.bugs_domain))
133-
134- if person.hide_email_addresses:
135- return format_address(
136- person.displayname,
137- "%s@%s" % (bug.id, config.launchpad.bugs_domain))
138-
139- if person.preferredemail is not None:
140- return format_address(person.displayname, person.preferredemail.email)
141-
142- # XXX: Bjorn Tillenius 2006-04-05:
143- # The person doesn't have a preferred email set, but they
144- # added a comment (either via the email UI, or because they were
145- # imported as a deaf reporter). It shouldn't be possible to use the
146- # email UI if you don't have a preferred email set, but work around
147- # it for now by trying hard to find the right email address to use.
148- email_addresses = shortlist(
149- getUtility(IEmailAddressSet).getByPerson(person))
150- if not email_addresses:
151- # XXX: Bjorn Tillenius 2006-05-21 bug=33427:
152- # A user should always have at least one email address,
153- # but due to bug #33427, this isn't always the case.
154- return format_address(person.displayname,
155- "%s@%s" % (bug.id, config.launchpad.bugs_domain))
156-
157- # At this point we have no validated emails to use: if any of the
158- # person's emails had been validated the preferredemail would be
159- # set. Since we have no idea of which email address is best to use,
160- # we choose the first one.
161- return format_address(person.displayname, email_addresses[0].email)
162+ displayname = 'Launchpad Bug Tracker'
163+ else:
164+ displayname = person.display_name
165+
166+ return format_address(displayname,
167+ "%s@%s" % (bug.id, config.launchpad.bugs_domain))
168
169
170 def get_bugmail_replyto_address(bug):

Subscribers

People subscribed via source and target branches