Change handling of rules for banned addresses.

If any of the senders are banned, both the member moderation and
nonmember moderation rules return False unconditionally.

Add test for the above.

2 files changed, 54 insertions, 41 deletions

diff --git a/src/mailman/rules/moderation.py b/src/mailman/rules/moderation.py
index d0852f2b2..355118857 100644
--- a/src/mailman/rules/moderation.py
+++ b/src/mailman/rules/moderation.py
@@ -42,12 +42,13 @@ class MemberModeration:
     def check(self, mlist, msg, msgdata):
         """See `IRule`."""
         user_manager = getUtility(IUserManager)
+        # The MemberModeration rule is unconditionally false if any of
+        # the senders are banned.
         for sender in msg.senders:
-            # The member-moderation rule is not set if the address is banned,
-            # even if it is linked to a subscribed addresses.
             if IBanManager(mlist).is_banned(sender):
                 return False
-            # Check for subscribed members linked to the address
+        for sender in msg.senders:
+            # Check for subscribed members linked to the address.
             member = mlist.members.get_member(sender)
             if member is None:
                 user = user_manager.get_user(sender)
@@ -55,7 +56,6 @@ class MemberModeration:
                     for address in user.addresses:
                         if mlist.members.get_member(address.email) is not None:
                             member = mlist.members.get_member(address.email)
-                            break
             if member is None:
                 return False
             action = (mlist.default_member_action
@@ -95,38 +95,36 @@ class NonmemberModeration:
     def check(self, mlist, msg, msgdata):
         """See `IRule`."""
         user_manager = getUtility(IUserManager)
+        # Initial check. If any of the senders are banned, we bail.
+        for sender in msg.senders:
+            if IBanManager(mlist).is_banned(sender):
+                return False
         # First ensure that all senders are already either members or
         # nonmembers.  If they are not subscribed in some role to the mailing
         # list, make them nonmembers.
-        # Maintain a record of which senders have linked subscribed users
-        found_linked_membership = {}
+        # Maintain a record of which senders have linked subscribed users.
+        found_linked_membership = set()
         for sender in msg.senders:
-            found_linked_membership[sender] = 'False'
-            # Check for linked user membership
             member = mlist.members.get_member(sender)
             if member is not None:
-                found_linked_membership[sender] = 'True'
+                found_linked_membership.add(sender)
             else:
                 user = user_manager.get_user(sender)
                 if user is not None:
                     for address in user.addresses:
                         if mlist.members.get_member(address.email) is not None:
-                            found_linked_membership[sender] = 'True'
-            # If the posting addres is banned, the post is not allowed
-            # to pass even if the address is linked to subscribed user.
-            if IBanManager(mlist).is_banned(sender):
-                found_linked_membership[sender] = 'False'
+                            found_linked_membership.add(sender)
             if (mlist.nonmembers.get_member(sender) is None and
-                    found_linked_membership.get(sender) == 'False'):
+                    sender not in found_linked_membership):
                 # The address is neither a member nor nonmember
-                # and has no linked subscribed user
+                # and has no linked subscribed user.
                 address = user_manager.get_address(sender)
                 assert address is not None, (
                     'Posting address is not registered: {}'.format(sender))
                 mlist.subscribe(address, MemberRole.nonmember)
-        # If a member is found, the member-moderation rule takes precedence.
+        # If a membership is found, the MemberModeration rule takes precedence.
         for sender in msg.senders:
-            if found_linked_membership.get(sender) == 'True':
+            if sender in found_linked_membership:
                 return False
         # Do nonmember moderation check.
         for sender in msg.senders:
diff --git a/src/mailman/rules/tests/test_moderation.py b/src/mailman/rules/tests/test_moderation.py
index 3ff18cd2e..7da07b200 100644
--- a/src/mailman/rules/tests/test_moderation.py
+++ b/src/mailman/rules/tests/test_moderation.py
@@ -211,16 +211,14 @@ A message body.
         self.assertEqual(msgdata.get('moderation_action'), 'hold')
 
     def test_linked_address_nonmembermoderation(self):
-        self._mlist.default_member_action = Action.accept
-        self._mlist.default_nonmember_action = Action.hold
         user_manager = getUtility(IUserManager)
-        kane = user_manager.create_user('kane@example.com')
-        set_preferred(kane)
-        self._mlist.subscribe(kane, MemberRole.member)
-        kane.link(user_manager.create_address('kane2@example.com'))
+        anne = user_manager.create_user('anne@example.com')
+        set_preferred(anne)
+        self._mlist.subscribe(anne, MemberRole.member)
+        anne.link(user_manager.create_address('anne2@example.com'))
         rule = moderation.NonmemberModeration()
         msg = mfs("""\
-From: kane2@example.com
+From: anne2@example.com
 To: test@example.com
 Subject: A test message
 Message-ID: <ant>
@@ -236,15 +234,14 @@ A message body.
 
     def test_linked_address_membermoderation(self):
         self._mlist.default_member_action = Action.accept
-        self._mlist.default_nonmember_action = Action.hold
         user_manager = getUtility(IUserManager)
-        kane = user_manager.create_user('kane@example.com')
-        set_preferred(kane)
-        self._mlist.subscribe(kane, MemberRole.member)
-        kane.link(user_manager.create_address('kane2@example.com'))
+        anne = user_manager.create_user('anne@example.com')
+        set_preferred(anne)
+        self._mlist.subscribe(anne, MemberRole.member)
+        anne.link(user_manager.create_address('anne2@example.com'))
         rule = moderation.MemberModeration()
         msg = mfs("""\
-From: kane2@example.com
+From: anne2@example.com
 To: test@example.com
 Subject: A test message
 Message-ID: <ant>
@@ -258,18 +255,16 @@ A message body.
         result = rule.check(self._mlist, msg, msgdata)
         self.assertTrue(result)
 
-    def test_banned_address_linked_to_user_moderation(self):
-        self._mlist.default_member_action = Action.accept
-        self._mlist.default_nonmember_action = Action.hold
+    def test_banned_address_linked_to_user(self):
         user_manager = getUtility(IUserManager)
-        kane = user_manager.create_user('kane@example.com')
-        set_preferred(kane)
-        self._mlist.subscribe(kane, MemberRole.member)
-        kane.link(user_manager.create_address('kane2@example.com'))
-        IBanManager(self._mlist).ban('kane2@example.com')
+        anne = user_manager.create_user('anne@example.com')
+        set_preferred(anne)
+        self._mlist.subscribe(anne, MemberRole.member)
+        anne.link(user_manager.create_address('anne2@example.com'))
+        IBanManager(self._mlist).ban('anne2@example.com')
         rule = moderation.MemberModeration()
         msg = mfs("""\
-From: kane2@example.com
+From: anne2@example.com
 To: test@example.com
 Subject: A test message
 Message-ID: <ant>
@@ -282,4 +277,24 @@ A message body.
         self.assertFalse(result)
         rule = moderation.NonmemberModeration()
         result = rule.check(self._mlist, msg, msgdata)
-        self.assertTrue(result)
+        self.assertFalse(result)
+
+    def test_banned_sender_among_multiple_senders(self):
+        user_manager = getUtility(IUserManager)
+        user_manager.create_address('ted@example.com')
+        user_manager.create_address('cris@example.com')
+        IBanManager(self._mlist).ban('cris@example.com')
+        rule = moderation.NonmemberModeration()
+        msg = mfs("""\
+From: ted@example.com
+Sender: cris@example.com
+To: test@example.com
+Subject: A test message
+Message-ID: <ant>
+MIME-Version: 1.0
+
+A message body.
+""")
+        msgdata = {}
+        result = rule.check(self._mlist, msg, msgdata)
+        self.assertFalse(result)