Merge lp:~mlm-author/mailman/2.1-author into lp:mailman/2.1

Proposed by Franck
Status: Merged
Merge reported by: Mark Sapiro
Merged at revision: not available
Proposed branch: lp:~mlm-author/mailman/2.1-author
Merge into: lp:mailman/2.1
Diff against target: 176 lines (+47/-3) (has conflicts)
10 files modified
Mailman/Defaults.py.in (+3/-0)
Mailman/Gui/General.py (+6/-0)
Mailman/Handlers/Cleanse.py (+12/-1)
Mailman/Handlers/CookHeaders.py (+1/-1)
Mailman/MailList.py (+1/-0)
Mailman/versions.py (+4/-0)
NEWS (+8/-0)
contrib/majordomo2mailman.pl (+1/-0)
doc/mailman-admin.txt (+6/-1)
doc/mailman-admin/node10.html (+5/-0)
Text conflict in Mailman/versions.py
Text conflict in NEWS
To merge this branch: bzr merge lp:~mlm-author/mailman/2.1-author
Reviewer Review Type Date Requested Status
Mark Sapiro code Needs Information
Review via email: mp+115035@code.launchpad.net

Description of the change

This change is optional through the use of the author_list variable and is set to off by default. Once set to on, it will use the list address in the From: header changing the display name to indicate who is the author of the post. The email address of the original post will be added to the reply-to field.

Overall, this change allows mailing lists that choose so, to be able to be compatible with authentication schemes such as ADSP or DMARC.

A test list using this branch is at http://lists.peachymango.org/mailman/listinfo/mlm-auth

To post a comment you must log in.
Revision history for this message
Mark Sapiro (msapiro) wrote :

Sorry for not responding to this sooner. Somehow I overlooked the original Mailman-coders notification.

The branch covers almost everything, but it is incomplete:

Mailman.versions.NewVars() needs
        add_only_if_missing('author_list', 0)

Mailman.Version.DATA_FILE_VERSION needs to be incremented.

Most importantly, the code in Cleanse.py has issues:
+ # We change the from so the list takes ownership of the email
+ if mlist.author_list:
+ mlist.include_sender_header = 0
+ if msg['reply-to'] == "" :
+ msg['reply-to'] = msg['reply-to'] + " , " + msg['from']
+ else:
+ msg['reply-to'] = msg['from']

The above test is backwards and it doesn't take into account the fact that msg['reply-to'] is not a null string, but is None if there is no Reply-To: It should be something like:
        if msg['reply-to']:
            msg['reply-to'] = msg['reply-to'] + " , " + msg['from']
        else:
           msg['reply-to'] = msg['from']

+ realname, email = parseaddr(msg['from'])
+ del msg['from']
+ msg['From'] = formataddr(('%s via %s' % (realname,mlist.real_name), mlist.GetListEmail()))
+ del msg['domainkey-signature']
+ del msg['dkim-signature']

The above two headers and any Authentication-Results: header will be removed in current Mailman if REMOVE_DKIM_HEADERS is set to Yes in mm_cfg.py. You might want to rely on that instead of removing them here, particularly because your documentation in Defaults.py.in doesn't mention these.

+ del msg['sender']

In any case, Since this can be merged without changing existing behavior, I'll consider doing it for 2.1.16

review: Needs Fixing
lp:~mlm-author/mailman/2.1-author updated
1341. By Franck

fixes for better merging in main branch

Revision history for this message
Franck (franckm) wrote :

I fixed the code as indicated, the conflicts seems cosmetic.

I removed the deletion of the DKIM and DK headers as there is a setting in cfg for that. Besides they don't create problems as the email can have another valid DKIM header using the domain of the list and the original headers can provide information on the path of the message.

Revision history for this message
Mark Sapiro (msapiro) wrote :

On 07/15/2013 02:35 AM, Franck wrote:
> You have been requested to review the proposed merge of lp:~mlm-author/mailman/2.1-author into lp:mailman/2.1.
>
> For more details, see:
> https://code.launchpad.net/~mlm-author/mailman/2.1-author/+merge/115035

This has turned into a major can of worms. I have attached the patch
that I have so far. Note that I haven't tested it yet. Also, the changes
to the mailman-admin manual aren't included here because those files are
actually derived from
<http://bazaar.launchpad.net/~mailman-administrivia/mailman-administrivia/admin/view/head:/doc/mailman-admin.tex>.

I still have some issues. I have actually changed the semantics of
REMOVE_DKIM_HEADERS so that current behavior is preserved, but there is
no a third option to remove the headers only for author_is_list lists
(and I changed that name to make it more meaningful, at least to me).

My one remaining serious question is the behavior for Sender: I notice
you both remove the original Sender: and set mlist.include_sender_header
= 0. This will actually change the list setting for
include_sender_header. because IncomingRunner will save the updated list
object when it finishes processing the pipeline. I'm not sure this is
appropriate. I think it should be up to the list admin to set this
behavior for the list and not have his choice changed behind his back.
What do you think about that?

Also, I think the code in Cleanse.py really belongs in CookHeaders.py,
but it's tricky because of 'what has precedence' questions, so I didn't
make that change. Another way to do this would be to add another handler
just to do the author_is_list stuff.

--
Mark Sapiro <email address hidden> The highway is for gamblers,
San Francisco Bay Area, California better use your sense - B. Dylan

Revision history for this message
Mark Sapiro (msapiro) wrote :
Download full text (9.7 KiB)

My comment is above (via email) The attached diff got stripped so I'm pasting it here in hopes it will survive:

=== modified file 'Mailman/Defaults.py.in'
--- Mailman/Defaults.py.in 2013-05-20 15:19:19 +0000
+++ Mailman/Defaults.py.in 2013-07-19 00:16:57 +0000
@@ -1,6 +1,6 @@
 # -*- python -*-

-# Copyright (C) 1998-2012 by the Free Software Foundation, Inc.
+# Copyright (C) 1998-2013 by the Free Software Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -548,7 +548,10 @@
 # footer or scrubbing attachments or even reply-to munging can break these
 # signatures. It is generally felt that these signatures have value, even if
 # broken and even if the outgoing message is resigned. However, some sites
-# may wish to remove these headers by setting this to Yes.
+# may wish to remove these headers. Possible values and meanings are:
+# No, 0, False -> do not remove headers.
+# 1 -> remove headers only if the list's author_is_list setting is Yes.
+# Yes, 2, True -> always remove headers.
 REMOVE_DKIM_HEADERS = No

 # All `normal' messages which are delivered to the entire list membership go
@@ -1065,6 +1068,10 @@
 # Send goodbye messages to unsubscribed members?
 DEFAULT_SEND_GOODBYE_MSG = Yes

+# Rewrite the From: header of posts replacing the posters address with
+# that of the list. Also see REMOVE_DKIM_HEADERS above.
+DEFAULT_AUTHOR_IS_LIST = No
+
 # Wipe sender information, and make it look like the list-admin
 # address sends all messages
 DEFAULT_ANONYMOUS_LIST = No

=== modified file 'Mailman/Gui/General.py'
--- Mailman/Gui/General.py 2011-10-04 21:53:13 +0000
+++ Mailman/Gui/General.py 2013-07-19 00:14:55 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2001-2011 by the Free Software Foundation, Inc.
+# Copyright (C) 2001-2013 by the Free Software Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -154,6 +154,14 @@
                             (listname %%05d) -> (listname 00123)
              """)),

+ ('author_is_list', mm_cfg.Radio, (_('No'), _('Yes')), 0,
+ _("""Replace the sender with the list address to conform with
+ policies like ADSP and DMARC. It replaces the poster's address
+ in the From: header with the list address and adds the poster to
+ the Reply-To: header, but the anonymous_list and Reply-To: header
+ munging settings below take priority. If setting this to Yes,
+ it is advised to set the MTA to DKIM sign all emails.""")),
+
             ('anonymous_list', mm_cfg.Radio, (_('No'), _('Yes')), 0,
              _("""Hide the sender of a message, replacing it with the list
              address (Removes From, Sender and Reply-To fields)""")),

=== modified file 'Mailman/Handlers/Cleanse.py'
--- Mailman/Handlers/Cleanse.py 2010-04-09 20:17:07 +0000
+++ Mailman/Handlers/Cleanse.py 2013-07-19 00:18:16 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 1998-2010 by the Free Software Foundation, Inc.
+# Copyright (C) 1998-2013 by the Free Software Foundation, Inc....

Read more...

review: Needs Information (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Mailman/Defaults.py.in'
2--- Mailman/Defaults.py.in 2013-05-20 15:19:19 +0000
3+++ Mailman/Defaults.py.in 2013-07-15 08:18:27 +0000
4@@ -1065,6 +1065,9 @@
5 # Send goodbye messages to unsubscribed members?
6 DEFAULT_SEND_GOODBYE_MSG = Yes
7
8+# Change emails so that the list is the author of emails
9+DEFAULT_AUTHOR_LIST = No
10+
11 # Wipe sender information, and make it look like the list-admin
12 # address sends all messages
13 DEFAULT_ANONYMOUS_LIST = No
14
15=== modified file 'Mailman/Gui/General.py'
16--- Mailman/Gui/General.py 2011-10-04 21:53:13 +0000
17+++ Mailman/Gui/General.py 2013-07-15 08:18:27 +0000
18@@ -154,6 +154,12 @@
19 (listname %%05d) -> (listname 00123)
20 """)),
21
22+ ('author_list', mm_cfg.Radio, (_('No'), _('Yes')), 0,
23+ _("""Replace the sender with the list address to conform with policies
24+ like ADSP and DMARC. It replaces the From to put the list address
25+ and modify the Reply-To to keep the reply to post behavior.
26+ It is advised to set the MTA to DKIM sign all emails.""")),
27+
28 ('anonymous_list', mm_cfg.Radio, (_('No'), _('Yes')), 0,
29 _("""Hide the sender of a message, replacing it with the list
30 address (Removes From, Sender and Reply-To fields)""")),
31
32=== modified file 'Mailman/Handlers/Cleanse.py'
33--- Mailman/Handlers/Cleanse.py 2010-04-09 20:17:07 +0000
34+++ Mailman/Handlers/Cleanse.py 2013-07-15 08:18:27 +0000
35@@ -19,7 +19,7 @@
36
37 import re
38
39-from email.Utils import formataddr
40+from email.Utils import formataddr, parseaddr
41
42 from Mailman.Utils import unique_message_id
43 from Mailman.Logging.Syslog import syslog
44@@ -38,6 +38,17 @@
45 del msg['x-approve']
46 # Also remove this header since it can contain a password
47 del msg['urgent']
48+ # We change the from so the list takes ownership of the email
49+ if mlist.author_list:
50+ mlist.include_sender_header = 0
51+ if msg['reply-to'] :
52+ msg['reply-to'] = msg['reply-to'] + " , " + msg['from']
53+ else:
54+ msg['reply-to'] = msg['from']
55+ realname, email = parseaddr(msg['from'])
56+ del msg['from']
57+ msg['From'] = formataddr(('%s via %s' % (realname,mlist.real_name), mlist.GetListEmail()))
58+ del msg['sender']
59 # We remove other headers from anonymous lists
60 if mlist.anonymous_list:
61 syslog('post', 'post to %s from %s anonymized',
62
63=== modified file 'Mailman/Handlers/CookHeaders.py'
64--- Mailman/Handlers/CookHeaders.py 2012-06-20 23:32:30 +0000
65+++ Mailman/Handlers/CookHeaders.py 2013-07-15 08:18:27 +0000
66@@ -159,7 +159,7 @@
67 # Also skip Cc if this is an anonymous list as list posting address
68 # is already in From and Reply-To in this case.
69 if mlist.personalize == 2 and mlist.reply_goes_to_list <> 1 \
70- and not mlist.anonymous_list:
71+ and not mlist.anonymous_list and not mlist.author_list:
72 # Watch out for existing Cc headers, merge, and remove dups. Note
73 # that RFC 2822 says only zero or one Cc header is allowed.
74 new = []
75
76=== modified file 'Mailman/MailList.py'
77--- Mailman/MailList.py 2012-06-20 23:32:30 +0000
78+++ Mailman/MailList.py 2013-07-15 08:18:27 +0000
79@@ -347,6 +347,7 @@
80 self.bounce_matching_headers = \
81 mm_cfg.DEFAULT_BOUNCE_MATCHING_HEADERS
82 self.header_filter_rules = []
83+ self.author_list = mm_cfg.DEFAULT_AUTHOR_LIST
84 self.anonymous_list = mm_cfg.DEFAULT_ANONYMOUS_LIST
85 internalname = self.internal_name()
86 self.real_name = internalname[0].upper() + internalname[1:]
87
88=== modified file 'Mailman/Version.py'
89=== modified file 'Mailman/versions.py'
90--- Mailman/versions.py 2012-06-20 23:32:30 +0000
91+++ Mailman/versions.py 2013-07-15 08:18:27 +0000
92@@ -416,8 +416,12 @@
93 mm_cfg.DEFAULT_REGULAR_EXCLUDE_LISTS)
94 add_only_if_missing('regular_include_lists',
95 mm_cfg.DEFAULT_REGULAR_INCLUDE_LISTS)
96+<<<<<<< TREE
97 add_only_if_missing('regular_exclude_ignore',
98 mm_cfg.DEFAULT_REGULAR_EXCLUDE_IGNORE)
99+=======
100+ add_only_if_missing('author_list', 0)
101+>>>>>>> MERGE-SOURCE
102
103
104
105
106
107=== modified file 'NEWS'
108--- NEWS 2013-07-14 09:10:07 +0000
109+++ NEWS 2013-07-15 08:18:27 +0000
110@@ -5,6 +5,7 @@
111
112 Here is a history of user visible changes to Mailman.
113
114+<<<<<<< TREE
115 2.1.16rc1 (14-Aug-2013)
116
117 New Features
118@@ -124,6 +125,13 @@
119 HTML entities. (LP: #1018208)
120
121 2.1.15 (13-Jun-2012)
122+=======
123+This Branch
124+ -Adding author_list to enable the application of ADSP and DMARC like
125+ email policies
126+
127+2.1.15 (xx-xxx-xxxx)
128+>>>>>>> MERGE-SOURCE
129
130 Security
131
132
133=== modified file 'contrib/majordomo2mailman.pl'
134--- contrib/majordomo2mailman.pl 2003-01-02 05:25:50 +0000
135+++ contrib/majordomo2mailman.pl 2013-07-15 08:18:27 +0000
136@@ -480,6 +480,7 @@
137 'max_num_recipients', "10",
138 'forbidden_posters', "[]",
139 'bounce_matching_headers', "\"\"\"\n\"\"\"\n",
140+ 'author_list', "0",
141 'anonymous_list', "0",
142 'nondigestable', "1",
143 'digestable', "1",
144
145=== modified file 'doc/mailman-admin.txt'
146--- doc/mailman-admin.txt 2013-07-14 09:10:07 +0000
147+++ doc/mailman-admin.txt 2013-07-15 08:18:27 +0000
148@@ -329,7 +329,12 @@
149 language. In this case, because of vagarities of the email
150 standards, you may or may not want to add a trailing space.
151
152- anonymous_list
153+ author_list
154+ This variable allows you to replace the From header with the
155+ list address, so that policies like ADSP or DMARC can be
156+ applied.
157+
158+ anonymous_list
159 This variable allows you to turn on some simple anonymizing
160 features of Mailman. When you set this option to Yes, Mailman
161 will remove or replace the From:, Sender:, and Reply-To: fields
162
163=== modified file 'doc/mailman-admin/node10.html'
164--- doc/mailman-admin/node10.html 2013-07-14 09:10:07 +0000
165+++ doc/mailman-admin/node10.html 2013-07-15 08:18:27 +0000
166@@ -140,6 +140,11 @@
167
168 <p>
169 </dd>
170+<dt><strong>author_list</strong></dt>
171+<dd>This variable allows you to replace the From header with the
172+ list address, so that policies like ADSP or DMARC can be
173+ applied.
174+</dd>
175 <dt><strong>anonymous_list</strong></dt>
176 <dd>This variable allows you to turn on some simple anonymizing
177 features of Mailman. When you set this option to <em>Yes</em>,