Merge branch 'abompard/mailman-fix-import21'

4 files changed, 198 insertions, 37 deletions

diff --git a/src/mailman/rules/moderation.py b/src/mailman/rules/moderation.py
index 0cc0b81c3..215a4c852 100644
--- a/src/mailman/rules/moderation.py
+++ b/src/mailman/rules/moderation.py
@@ -23,6 +23,8 @@ __all__ = [
     ]
 
 
+import re
+
 from mailman.core.i18n import _
 from mailman.interfaces.action import Action
 from mailman.interfaces.member import MemberRole
@@ -64,6 +66,12 @@ class MemberModeration:
 
 
 
+def _record_action(msgdata, action, sender, reason):
+    msgdata['moderation_action'] = action
+    msgdata['moderation_sender'] = sender
+    msgdata.setdefault('moderation_reasons', []).append(reason)
+
+
 @implementer(IRule)
 class NonmemberModeration:
     """The nonmember moderation rule."""
@@ -93,19 +101,33 @@ class NonmemberModeration:
         # Do nonmember moderation check.
         for sender in msg.senders:
             nonmember = mlist.nonmembers.get_member(sender)
-            action = (None if nonmember is None
-                      else nonmember.moderation_action)
+            assert nonmember is not None, (
+                'Sender not added to the nonmembers: {0}'.format(sender))
+            # Check the '*_these_nonmembers' properties first.  XXX These are
+            # legacy attributes from MM2.1; their database type is 'pickle' and
+            # they should eventually get replaced.
+            for action in ('accept', 'hold', 'reject', 'discard'):
+                legacy_attribute_name = '{}_these_nonmembers'.format(action)
+                checklist = getattr(mlist, legacy_attribute_name)
+                for addr in checklist:
+                    if ((addr.startswith('^') and re.match(addr, sender))
+                            or addr == sender):
+                        # The reason will get translated at the point of use.
+                        reason = 'The sender is in the nonmember {} list'
+                        _record_action(msgdata, action, sender,
+                                       reason.format(action))
+                        return True
+            action = nonmember.moderation_action
             if action is Action.defer:
                 # The regular moderation rules apply.
                 return False
             elif action is not None:
                 # We must stringify the moderation action so that it can be
                 # stored in the pending request table.
-                msgdata['moderation_action'] = action.name
-                msgdata['moderation_sender'] = sender
-                msgdata.setdefault('moderation_reasons', []).append(
-                    # This will get translated at the point of use.
-                    'The message is not from a list member')
+                #
+                # The reason will get translated at the point of use.
+                reason = 'The message is not from a list member'
+                _record_action(msgdata, action.name, sender, reason)
                 return True
         # The sender must be a member, so this rule does not match.
         return False
diff --git a/src/mailman/rules/tests/test_moderation.py b/src/mailman/rules/tests/test_moderation.py
index a2e988874..79aade587 100644
--- a/src/mailman/rules/tests/test_moderation.py
+++ b/src/mailman/rules/tests/test_moderation.py
@@ -110,3 +110,42 @@ A message body.
         reasons = msgdata['moderation_reasons']
         self.assertEqual(
             reasons, ['The message comes from a moderated member'])
+
+    def test_these_nonmembers(self):
+        # Test the legacy *_these_nonmembers attributes.
+        user_manager = getUtility(IUserManager)
+        actions = {
+            'anne@example.com': 'accept',
+            'bill@example.com': 'hold',
+            'chris@example.com': 'reject',
+            'dana@example.com': 'discard',
+            '^anne-.*@example.com': 'accept',
+            '^bill-.*@example.com': 'hold',
+            '^chris-.*@example.com': 'reject',
+            '^dana-.*@example.com': 'discard',
+            }
+        rule = moderation.NonmemberModeration()
+        user_manager = getUtility(IUserManager)
+        for address, action_name in actions.items():
+            setattr(self._mlist,
+                    '{}_these_nonmembers'.format(action_name),
+                    [address])
+            if address.startswith('^'):
+                # It's a pattern, craft a proper address.
+                address = address[1:].replace('.*', 'something')
+            user_manager.create_address(address)
+            msg = mfs("""\
+From: {}
+To: test@example.com
+Subject: A test message
+Message-ID: <ant>
+MIME-Version: 1.0
+
+A message body.
+""".format(address))
+            msgdata = {}
+            result = rule.check(self._mlist, msg, msgdata)
+            self.assertTrue(result, 'NonmemberModeration rule should hit')
+            self.assertIn('moderation_action', msgdata)
+            self.assertEqual(msgdata['moderation_action'], action_name,
+                'Wrong action for {}: {}'.format(address, action_name))
diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py
index b55d8c38d..293e9c39c 100644
--- a/src/mailman/utilities/importer.py
+++ b/src/mailman/utilities/importer.py
@@ -103,14 +103,9 @@ def filter_action_mapping(value):
         }[value]
 
 
-
-def member_action_mapping(value):
-    # The mlist.default_member_action and mlist.default_nonmember_action enum
-    # values are different in Mailman 2.1, because they have been merged into
-    # a single enum in Mailman 3.
-    #
-    # For default_member_action, which used to be called
-    # member_moderation_action, the values were: 0==Hold, 1=Reject, 2==Discard
+def member_moderation_action_mapping(value):
+    # Convert the member_moderation_action option to an Action enum.
+    # The values were: 0==Hold, 1==Reject, 2==Discard
     return {
         0: Action.hold,
         1: Action.reject,
@@ -129,7 +124,6 @@ def nonmember_action_mapping(value):
         3: Action.discard,
         }[value]
 
-
 
 def check_language_code(code):
     if code is None:
@@ -161,7 +155,6 @@ TYPES = dict(
     autoresponse_grace_period=days_to_delta,
     bounce_info_stale_after=seconds_to_delta,
     bounce_you_are_disabled_warnings_interval=seconds_to_delta,
-    default_member_action=member_action_mapping,
     default_nonmember_action=nonmember_action_mapping,
     digest_volume_frequency=DigestFrequency,
     filter_action=filter_action_mapping,
@@ -190,7 +183,6 @@ NAME_MAPPINGS = dict(
     filter_mime_types='filter_types',
     generic_nonmember_action='default_nonmember_action',
     include_list_post_header='allow_list_posts',
-    member_moderation_action='default_member_action',
     mod_password='moderator_password',
     news_moderation='newsgroup_moderation',
     news_prefix_subject_too='nntp_prefix_subject_too',
@@ -268,6 +260,22 @@ def import_config_pck(mlist, config_dict):
             setattr(mlist, 'last_post_at', value)
             continue
         setattr(mlist, key, value)
+    # Handle the moderation policy.
+    #
+    # The mlist.default_member_action and mlist.default_nonmember_action enum
+    # values are different in Mailman 2.1, because they have been merged into a
+    # single enum in Mailman 3.
+    #
+    # Unmoderated lists used to have default_member_moderation set to a false
+    # value; this translates to the Defer default action.  Moderated lists with
+    # the default_member_moderation set to a true value used to store the
+    # action in the member_moderation_action flag, the values were: 0==Hold,
+    # 1=Reject, 2==Discard
+    if bool(config_dict.get('default_member_moderation', 0)):
+        mlist.default_member_action = member_moderation_action_mapping(
+            config_dict.get('member_moderation_action'))
+    else:
+        mlist.default_member_action = Action.defer
     # Handle the archiving policy.  In MM2.1 there were two boolean options
     # but only three of the four possible states were valid.  Now there's just
     # an enum.
@@ -399,12 +407,25 @@ def import_config_pck(mlist, config_dict):
                       MemberRole.owner)
         import_roster(mlist, config_dict, config_dict.get('moderator', []),
                       MemberRole.moderator)
+        # Now import the '*_these_nonmembers' properties, filtering out the
+        # regexps which will remain in the property.
+        for action_name in ('accept', 'hold', 'reject', 'discard'):
+            prop_name = '{}_these_nonmembers'.format(action_name)
+            emails = [addr
+                      for addr in config_dict.get(prop_name, [])
+                      if not addr.startswith('^')]
+            import_roster(mlist, config_dict, emails, MemberRole.nonmember,
+                          Action[action_name])
+            # Only keep the regexes in the legacy list property.
+            list_prop = getattr(mlist, prop_name)
+            for email in emails:
+                list_prop.remove(email)
     finally:
         mlist.send_welcome_message = send_welcome_message
 
 
 
-def import_roster(mlist, config_dict, members, role):
+def import_roster(mlist, config_dict, members, role, action=None):
     """Import members lists from a config.pck configuration dictionary.
 
     :param mlist: The mailing list.
@@ -415,6 +436,8 @@ def import_roster(mlist, config_dict, members, role):
     :type members: list
     :param role: The MemberRole to import them as.
     :type role: MemberRole enum
+    :param action: The default nonmember action.
+    :type action: Action
     """
     usermanager = getUtility(IUserManager)
     validator = getUtility(IEmailValidator)
@@ -447,14 +470,13 @@ def import_roster(mlist, config_dict, members, role):
                 address = usermanager.create_address(original_email)
                 address.verified_on = datetime.datetime.now()
             user.link(address)
-        mlist.subscribe(address, role)
-        member = roster.get_member(email)
+        member = mlist.subscribe(address, role)
         assert member is not None
-        prefs = config_dict.get('user_options', {}).get(email, 0)
+        prefs = config_dict.get('user_options', {}).get(email)
         if email in config_dict.get('members', {}):
             member.preferences.delivery_mode = DeliveryMode.regular
         elif email in config_dict.get('digest_members', {}):
-            if prefs & 8: # DisableMime
+            if prefs is not None and prefs & 8:               # DisableMime
                 member.preferences.delivery_mode = \
                   DeliveryMode.plaintext_digests
             else:
@@ -488,15 +510,26 @@ def import_roster(mlist, config_dict, members, role):
         elif oldds == 4:
             member.preferences.delivery_status = DeliveryStatus.by_bounces
         # Moderation.
-        if prefs & 128:
-            member.moderation_action = Action.hold
+        if prefs is not None:
+            # We're adding a member.
+            if prefs & 128:
+                # The member is moderated.  Check the member_moderation_action
+                # option to know which action should be taken.
+                action = member_moderation_action_mapping(
+                    config_dict.get("member_moderation_action"))
+            else:
+                action = Action.accept
+        if action is not None:
+            # Either this was set right above or in the function's arguments
+            # for nonmembers.
+            member.moderation_action = action
         # Other preferences.
-        #
-        # AcknowledgePosts
-        member.preferences.acknowledge_posts = bool(prefs & 4)
-        # ConcealSubscription
-        member.preferences.hide_address = bool(prefs & 16)
-        # DontReceiveOwnPosts
-        member.preferences.receive_own_postings = not bool(prefs & 2)
-        # DontReceiveDuplicates
-        member.preferences.receive_list_copy = not bool(prefs & 256)
+        if prefs is not None:
+            # AcknowledgePosts
+            member.preferences.acknowledge_posts = bool(prefs & 4)
+            # ConcealSubscription
+            member.preferences.hide_address = bool(prefs & 16)
+            # DontReceiveOwnPosts
+            member.preferences.receive_own_postings = not bool(prefs & 2)
+            # DontReceiveDuplicates
+            member.preferences.receive_list_copy = not bool(prefs & 256)
diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py
index 9f3d59d5a..a0b1767c1 100644
--- a/src/mailman/utilities/tests/test_import.py
+++ b/src/mailman/utilities/tests/test_import.py
@@ -444,15 +444,26 @@ class TestMemberActionImport(unittest.TestCase):
         for key, value in expected.items():
             self.assertEqual(getattr(self._mlist, key), value)
 
+    def test_member_defer(self):
+        # If default_member_moderation is not set, the member_moderation_action
+        # value is meaningless.
+        self._pckdict['default_member_moderation'] = 0
+        for mmaval in range(3):
+            self._pckdict['member_moderation_action'] = mmaval
+            self._do_test(dict(default_member_action=Action.defer))
+
     def test_member_hold(self):
+        self._pckdict['default_member_moderation'] = 1
         self._pckdict['member_moderation_action'] = 0
         self._do_test(dict(default_member_action=Action.hold))
 
     def test_member_reject(self):
+        self._pckdict['default_member_moderation'] = 1
         self._pckdict['member_moderation_action'] = 1
         self._do_test(dict(default_member_action=Action.reject))
 
     def test_member_discard(self):
+        self._pckdict['default_member_moderation'] = 1
         self._pckdict['member_moderation_action'] = 2
         self._do_test(dict(default_member_action=Action.discard))
 
@@ -637,6 +648,22 @@ class TestRosterImport(unittest.TestCase):
                 'bob@example.com',
                 'fred@example.com',
             ],
+            'accept_these_nonmembers': [
+                'gene@example.com',
+                '^gene-.*@example.com',
+            ],
+            'hold_these_nonmembers': [
+                'homer@example.com',
+                '^homer-.*@example.com',
+            ],
+            'reject_these_nonmembers': [
+                'iris@example.com',
+                '^iris-.*@example.com',
+            ],
+            'discard_these_nonmembers': [
+                'kenny@example.com',
+                '^kenny-.*@example.com',
+            ],
         }
         self._usermanager = getUtility(IUserManager)
         language_manager = getUtility(ILanguageManager)
@@ -820,6 +847,28 @@ class TestRosterImport(unittest.TestCase):
                                  queue, file_count))
         self.assertTrue(self._mlist.send_welcome_message)
 
+    def test_nonmembers(self):
+        import_config_pck(self._mlist, self._pckdict)
+        expected = {
+            'gene': Action.accept,
+            'homer': Action.hold,
+            'iris': Action.reject,
+            'kenny': Action.discard,
+            }
+        for name, action in expected.items():
+            self.assertIn('{}@example.com'.format(name),
+                          [a.email for a in self._mlist.nonmembers.addresses],
+                          'Address {} was not imported'.format(name))
+            member = self._mlist.nonmembers.get_member(
+                '{}@example.com'.format(name))
+            self.assertEqual(member.moderation_action, action)
+            # Only regexps should remain in the list property.
+            list_prop = getattr(
+                self._mlist,
+                '{}_these_nonmembers'.format(action.name))
+            self.assertEqual(len(list_prop), 1)
+            self.assertTrue(all(addr.startswith('^') for addr in list_prop))
+
 
 
 class TestPreferencesImport(unittest.TestCase):
@@ -902,11 +951,29 @@ class TestPreferencesImport(unittest.TestCase):
             self.assertEqual(member.delivery_status, expected)
             member.unsubscribe()
 
-    def test_moderate(self):
-        # Option flag Moderate is translated to
-        # member.moderation_action = Action.hold
+    def test_moderate_hold(self):
+        # Option flag Moderate is translated to the action set in
+        # member_moderation_action.
+        self._pckdict['member_moderation_action'] = 0
         self._do_test(128, dict(moderation_action=Action.hold))
 
+    def test_moderate_reject(self):
+        # Option flag Moderate is translated to the action set in
+        # member_moderation_action.
+        self._pckdict['member_moderation_action'] = 1
+        self._do_test(128, dict(moderation_action=Action.reject))
+
+    def test_moderate_hold_discard(self):
+        # Option flag Moderate is translated to the action set in
+        # member_moderation_action.
+        self._pckdict['member_moderation_action'] = 2
+        self._do_test(128, dict(moderation_action=Action.discard))
+
+    def test_no_moderate(self):
+        # If option flag Moderate is not set, action is accept
+        self._pckdict['member_moderation_action'] = 1 # reject
+        self._do_test(0, dict(moderation_action=Action.accept))
+
     def test_multiple_options(self):
         # DontReceiveDuplicates & DisableMime & SuppressPasswordReminder
         # Keys might be Python 2 str/bytes or unicode.