11 files changed, 461 insertions, 38 deletions

diff --git a/src/mailman/Utils.py b/src/mailman/Utils.py
index 49b7d082e..e10a9fde5 100644
--- a/src/mailman/Utils.py
+++ b/src/mailman/Utils.py
@@ -407,29 +407,3 @@ def strip_verbose_pattern(pattern):
                 newpattern += c
         i += 1
     return newpattern
-
-
-
-def get_pattern(email, pattern_list):
-    """Returns matched entry in pattern_list if email matches.
-    Otherwise returns None.
-    """
-    if not pattern_list:
-        return None
-    matched = None
-    for pattern in pattern_list:
-        if pattern.startswith('^'):
-            # This is a regular expression match
-            try:
-                if re.search(pattern, email, re.IGNORECASE):
-                    matched = pattern
-                    break
-            except re.error:
-                # BAW: we should probably remove this pattern
-                pass
-        else:
-            # Do the comparison case insensitively
-            if pattern.lower() == email.lower():
-                matched = pattern
-                break
-    return matched
diff --git a/src/mailman/app/docs/bans.rst b/src/mailman/app/docs/bans.rst
new file mode 100644
index 000000000..bb6d5902e
--- /dev/null
+++ b/src/mailman/app/docs/bans.rst
@@ -0,0 +1,167 @@
+=======================
+Banning email addresses
+=======================
+
+Email addresses can be banned from ever subscribing, either to a specific
+mailing list or globally within the Mailman system.  Both explicit email
+addresses and email address patterns can be banned.
+
+Bans are managed through the `Ban Manager`.
+
+    >>> from zope.component import getUtility
+    >>> from mailman.interfaces.bans import IBanManager
+    >>> ban_manager = getUtility(IBanManager)
+
+At first, no email addresses are banned, either globally...
+
+    >>> ban_manager.is_banned('anne@example.com')
+    False
+
+...or for a specific mailing list.
+
+    >>> ban_manager.is_banned('bart@example.com', 'test@example.com')
+    False
+
+
+Specific bans
+=============
+
+An email address can be banned from a specific mailing list by adding a ban to
+the ban manager.
+
+    >>> ban_manager.ban('cris@example.com', 'test@example.com')
+    >>> ban_manager.is_banned('cris@example.com', 'test@example.com')
+    True
+    >>> ban_manager.is_banned('bart@example.com', 'test@example.com')
+    False
+
+However, this is not a global ban.
+
+    >>> ban_manager.is_banned('cris@example.com')
+    False
+
+
+Global bans
+===========
+
+An email address can be banned globally, so that it cannot be subscribed to
+any mailing list.
+
+    >>> ban_manager.ban('dave@example.com')
+
+Dave is banned from the test mailing list...
+
+    >>> ban_manager.is_banned('dave@example.com', 'test@example.com')
+    True
+
+...and the sample mailing list.
+
+    >>> ban_manager.is_banned('dave@example.com', 'sample@example.com')
+    True
+
+Dave is also banned globally.
+
+    >>> ban_manager.is_banned('dave@example.com')
+    True
+
+Cris however is not banned globally.
+
+    >>> ban_manager.is_banned('cris@example.com')
+    False
+
+Even though Cris is not banned globally, we can add a global ban for her.
+
+    >>> ban_manager.ban('cris@example.com')
+    >>> ban_manager.is_banned('cris@example.com')
+    True
+
+Cris is obviously still banned from specific mailing lists.
+
+    >>> ban_manager.is_banned('cris@example.com', 'test@example.com')
+    True
+    >>> ban_manager.is_banned('cris@example.com', 'sample@example.com')
+    True
+
+We can remove the global ban to once again just ban her address from the test
+list.
+
+    >>> ban_manager.unban('cris@example.com')
+    >>> ban_manager.is_banned('cris@example.com', 'test@example.com')
+    True
+    >>> ban_manager.is_banned('cris@example.com', 'sample@example.com')
+    False
+
+
+Regular expression bans
+=======================
+
+Entire email address patterns can be banned, both for a specific mailing list
+and globally, just as specific addresses can be banned.  Use this for example,
+when an entire domain is a spam faucet.  When using a pattern, the email
+address must start with a caret (^).
+
+    >>> ban_manager.ban('^.*@example.org', 'test@example.com')
+
+Now, no one from example.org can subscribe to the test list.
+
+    >>> ban_manager.is_banned('elle@example.org', 'test@example.com')
+    True
+    >>> ban_manager.is_banned('eperson@example.org', 'test@example.com')
+    True
+    >>> ban_manager.is_banned('elle@example.com', 'test@example.com')
+    False
+
+They are not, however banned globally.
+
+    >>> ban_manager.is_banned('elle@example.org', 'sample@example.com')
+    False
+    >>> ban_manager.is_banned('elle@example.org')
+    False
+
+Of course, we can ban everyone from example.org globally too.
+
+    >>> ban_manager.ban('^.*@example.org')
+    >>> ban_manager.is_banned('elle@example.org', 'sample@example.com')
+    True
+    >>> ban_manager.is_banned('elle@example.org')
+    True
+
+We can remove the mailing list ban on the pattern, though the global ban will
+still be in place.
+
+    >>> ban_manager.unban('^.*@example.org', 'test@example.com')
+    >>> ban_manager.is_banned('elle@example.org', 'test@example.com')
+    True
+    >>> ban_manager.is_banned('elle@example.org', 'sample@example.com')
+    True
+    >>> ban_manager.is_banned('elle@example.org')
+    True
+
+But once the global ban is removed, everyone from example.org can subscribe to
+the mailing lists.
+
+    >>> ban_manager.unban('^.*@example.org')
+    >>> ban_manager.is_banned('elle@example.org', 'test@example.com')
+    False
+    >>> ban_manager.is_banned('elle@example.org', 'sample@example.com')
+    False
+    >>> ban_manager.is_banned('elle@example.org')
+    False
+
+
+Adding and removing bans
+========================
+
+It is not an error to add a ban more than once.  These are just ignored.
+
+    >>> ban_manager.ban('fred@example.com', 'test@example.com')
+    >>> ban_manager.ban('fred@example.com', 'test@example.com')
+    >>> ban_manager.is_banned('fred@example.com', 'test@example.com')
+    True
+
+Nor is it an error to remove a ban more than once.
+
+    >>> ban_manager.unban('fred@example.com', 'test@example.com')
+    >>> ban_manager.unban('fred@example.com', 'test@example.com')
+    >>> ban_manager.is_banned('fred@example.com', 'test@example.com')
+    False
diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py
index 32ba1ff42..8723fd781 100644
--- a/src/mailman/app/membership.py
+++ b/src/mailman/app/membership.py
@@ -34,6 +34,7 @@ from mailman.app.notifications import send_goodbye_message
 from mailman.core.i18n import _
 from mailman.email.message import OwnerNotification
 from mailman.interfaces.address import IEmailValidator
+from mailman.interfaces.bans import IBanManager
 from mailman.interfaces.member import (
     AlreadySubscribedError, MemberRole, MembershipIsBannedError,
     NotAMemberError)
@@ -71,10 +72,8 @@ def add_member(mlist, email, realname, password, delivery_mode, language):
     if mlist.members.get_member(email) is not None:
         raise AlreadySubscribedError(
             mlist.fqdn_listname, email, MemberRole.member)
-    # Check for banned email addresses here too for administrative mass
-    # subscribes and confirmations.
-    pattern = Utils.get_pattern(email, mlist.ban_list)
-    if pattern:
+    # Check to see if the email address is banned.
+    if getUtility(IBanManager).is_banned(email, mlist.fqdn_listname):
         raise MembershipIsBannedError(mlist, email)
     # See if there's already a user linked with the given address.
     user_manager = getUtility(IUserManager)
diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py
index dc8009f65..b0e1bae5d 100644
--- a/src/mailman/app/tests/test_membership.py
+++ b/src/mailman/app/tests/test_membership.py
@@ -32,7 +32,8 @@ from zope.component import getUtility
 from mailman.app.lifecycle import create_list
 from mailman.app.membership import add_member
 from mailman.core.constants import system_preferences
-from mailman.interfaces.member import DeliveryMode
+from mailman.interfaces.bans import IBanManager
+from mailman.interfaces.member import DeliveryMode, MembershipIsBannedError
 from mailman.interfaces.usermanager import IUserManager
 from mailman.testing.helpers import reset_the_world
 from mailman.testing.layers import ConfigLayer
@@ -68,6 +69,60 @@ class AddMemberTest(unittest.TestCase):
         self.assertEqual(member.address.email, 'aperson@example.com')
         self.assertEqual(member.mailing_list, 'test@example.com')
 
+    def test_add_member_banned(self):
+        # Test that members who are banned by specific address cannot
+        # subscribe to the mailing list.
+        getUtility(IBanManager).ban('anne@example.com', 'test@example.com')
+        self.assertRaises(
+            MembershipIsBannedError,
+            add_member, self._mlist, 'anne@example.com', 'Anne Person',
+            '123', DeliveryMode.regular, system_preferences.preferred_language)
+
+    def test_add_member_globally_banned(self):
+        # Test that members who are banned by specific address cannot
+        # subscribe to the mailing list.
+        getUtility(IBanManager).ban('anne@example.com')
+        self.assertRaises(
+            MembershipIsBannedError,
+            add_member, self._mlist, 'anne@example.com', 'Anne Person',
+            '123', DeliveryMode.regular, system_preferences.preferred_language)
+
+    def test_add_member_banned_from_different_list(self):
+        # Test that members who are banned by specific address cannot
+        # subscribe to the mailing list.
+        getUtility(IBanManager).ban('anne@example.com', 'sample@example.com')
+        member = add_member(self._mlist, 'anne@example.com',
+                            'Anne Person', '123', DeliveryMode.regular,
+                            system_preferences.preferred_language)
+        self.assertEqual(member.address.email, 'anne@example.com')
+
+    def test_add_member_banned_by_pattern(self):
+        # Test that members who are banned by specific address cannot
+        # subscribe to the mailing list.
+        getUtility(IBanManager).ban('^.*@example.com', 'test@example.com')
+        self.assertRaises(
+            MembershipIsBannedError,
+            add_member, self._mlist, 'anne@example.com', 'Anne Person',
+            '123', DeliveryMode.regular, system_preferences.preferred_language)
+
+    def test_add_member_globally_banned_by_pattern(self):
+        # Test that members who are banned by specific address cannot
+        # subscribe to the mailing list.
+        getUtility(IBanManager).ban('^.*@example.com')
+        self.assertRaises(
+            MembershipIsBannedError,
+            add_member, self._mlist, 'anne@example.com', 'Anne Person',
+            '123', DeliveryMode.regular, system_preferences.preferred_language)
+
+    def test_add_member_banned_from_different_list_by_pattern(self):
+        # Test that members who are banned by specific address cannot
+        # subscribe to the mailing list.
+        getUtility(IBanManager).ban('^.*@example.com', 'sample@example.com')
+        member = add_member(self._mlist, 'anne@example.com',
+                            'Anne Person', '123', DeliveryMode.regular,
+                            system_preferences.preferred_language)
+        self.assertEqual(member.address.email, 'anne@example.com')
+
 
 
 def test_suite():
diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml
index bf42d6635..3b4497ab8 100644
--- a/src/mailman/config/configure.zcml
+++ b/src/mailman/config/configure.zcml
@@ -17,6 +17,11 @@
     />
 
   <utility
+    factory="mailman.model.bans.BanManager"
+    provides="mailman.interfaces.bans.IBanManager"
+    />
+
+  <utility
     factory="mailman.model.domain.DomainManager"
     provides="mailman.interfaces.domain.IDomainManager"
     />
diff --git a/src/mailman/database/mailman.sql b/src/mailman/database/mailman.sql
index bb7f9dc57..9d2b015b7 100644
--- a/src/mailman/database/mailman.sql
+++ b/src/mailman/database/mailman.sql
@@ -116,8 +116,7 @@ CREATE TABLE mailinglist (
         autorespond_requests INTEGER,
         autoresponse_request_text TEXT,
         autoresponse_grace_period TEXT,
-        -- Bounce and ban.
-        ban_list BLOB,
+        -- Bounces.
         bounce_info_stale_after TEXT,
         bounce_matching_headers TEXT,
         bounce_notify_owner_on_disable BOOLEAN,
@@ -270,3 +269,10 @@ CREATE INDEX ix_member_address_id ON member (address_id);
 CREATE INDEX ix_member_preferences_id ON member (preferences_id);
 CREATE INDEX ix_pendedkeyvalue_pended_id ON pendedkeyvalue (pended_id);
 CREATE INDEX ix_user_preferences_id ON user (preferences_id);
+
+CREATE TABLE ban (
+    id INTEGER NOT NULL,
+    email TEXT,
+    mailing_list TEXT,
+    PRIMARY KEY (id)
+);
diff --git a/src/mailman/interfaces/bans.py b/src/mailman/interfaces/bans.py
new file mode 100644
index 000000000..d14109cc1
--- /dev/null
+++ b/src/mailman/interfaces/bans.py
@@ -0,0 +1,110 @@
+# Copyright (C) 2011 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Manager of email address bans."""
+
+from __future__ import absolute_import, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'IBan',
+    'IBanManager',
+    ]
+
+
+from zope.interface import Attribute, Interface
+
+
+
+class IBan(Interface):
+    """A specific ban.
+
+    In general, this interface isn't publicly useful.
+    """
+
+    email = Attribute('The banned email address, or pattern.')
+
+    mailing_list = Attribute(
+        """The fqdn name of the mailing list the ban applies to.
+
+        Use None if this is a global ban.
+        """)
+
+
+
+class IBanManager(Interface):
+    """The global manager of email address bans."""
+
+    def ban(email, mailing_list=None):
+        """Ban an email address from subscribing to a mailing list.
+
+        When an email address is banned, it will not be allowed to subscribe
+        to a the named mailing list.  This does not affect any email address
+        already subscribed to the mailing list.  With the default arguments,
+        an email address can be banned globally from subscribing to any
+        mailing list on the system.
+
+        It is also possible to add a 'ban pattern' whereby all email addresses
+        matching a Python regular expression can be banned.  This is
+        accomplished by using a `^` as the first character in `email`.
+
+        When an email address is already banned for the given mailing list (or
+        globally), then this method does nothing.  However, it is possible to
+        extend a ban for a specific mailing list into a global ban; both bans
+        would be in place and they can be removed individually.
+
+        :param email: The text email address being banned or, if the string
+            starts with a caret (^), the email address pattern to ban.
+        :type email: str
+        :param mailing_list: The fqdn name of the mailing list to which the
+            ban applies.  If None, then the ban is global.
+        :type mailing_list: string
+        """
+
+    def unban(email, mailing_list=None):
+        """Remove an email address ban.
+
+        This removes a specific or global email address ban, which would have
+        been added with the `ban()` method.  If a ban is lifted which did not
+        exist, this method does nothing.
+
+        :param email: The text email address being unbanned or, if the string
+            starts with a caret (^), the email address pattern to unban.
+        :type email: str
+        :param mailing_list: The fqdn name of the mailing list to which the
+            unban applies.  If None, then the unban is global.
+        :type mailing_list: string
+        """
+
+    def is_banned(email, mailing_list=None):
+        """Check whether a specific email address is banned.
+
+        `email` must be a text email address; it cannot be a pattern.  The
+        given email address is checked against all registered bans, both
+        specific and regular expression, both for the named mailing list (if
+        given), and globally.
+
+        :param email: The text email address being checked.
+        :type email: str
+        :param mailing_list: The fqdn name of the mailing list being checked.
+            Note that if not None, both specific and global bans will be
+            checked.  If None, then only global bans will be checked.
+        :type mailing_list: string
+        :return: A flag indicating whether the given email address is banned
+            or not.
+        :rtype: bool
+        """
diff --git a/src/mailman/model/bans.py b/src/mailman/model/bans.py
new file mode 100644
index 000000000..95711cfa9
--- /dev/null
+++ b/src/mailman/model/bans.py
@@ -0,0 +1,110 @@
+# Copyright (C) 2011 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Ban manager."""
+
+from __future__ import absolute_import, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'BanManager',
+    ]
+
+
+import re
+
+from storm.locals import Int, Unicode
+from zope.interface import implements
+
+from mailman.config import config
+from mailman.database.model import Model
+from mailman.interfaces.bans import IBan, IBanManager
+
+
+
+class Ban(Model):
+    implements(IBan)
+
+    id = Int(primary=True)
+    email = Unicode()
+    mailing_list = Unicode()
+
+    def __init__(self, email, mailing_list):
+        super(Ban, self).__init__()
+        self.email = email
+        self.mailing_list = mailing_list
+
+
+
+class BanManager:
+    implements(IBanManager)
+
+    def ban(self, email, mailing_list=None):
+        """See `IBanManager`."""
+        bans = config.db.store.find(
+            Ban, email=email, mailing_list=mailing_list)
+        if bans.count() == 0:
+            ban = Ban(email, mailing_list)
+            config.db.store.add(ban)
+
+    def unban(self, email, mailing_list=None):
+        """See `IBanManager`."""
+        ban = config.db.store.find(
+            Ban, email=email, mailing_list=mailing_list).one()
+        if ban is not None:
+            config.db.store.remove(ban)
+
+    def is_banned(self, email, mailing_list=None):
+        """See `IBanManager`."""
+        # A specific mailing list ban is being checked, however the email
+        # address could be banned specifically, or globally.
+        if mailing_list is not None:
+            # Try specific bans first.
+            bans = config.db.store.find(
+                Ban, email=email, mailing_list=mailing_list)
+            if bans.count() > 0:
+                return True
+            # Try global bans next.
+            bans = config.db.store.find(Ban, email=email, mailing_list=None)
+            if bans.count() > 0:
+                return True
+            # Now try specific mailing list bans, but with a pattern.
+            bans = config.db.store.find(Ban, mailing_list=mailing_list)
+            for ban in bans:
+                if (ban.email.startswith('^') and
+                    re.match(ban.email, email, re.IGNORECASE) is not None):
+                    return True
+            # And now try global pattern bans.
+            bans = config.db.store.find(Ban, mailing_list=None)
+            for ban in bans:
+                if (ban.email.startswith('^') and
+                    re.match(ban.email, email, re.IGNORECASE) is not None):
+                    return True
+        else:
+            # The client is asking for global bans.  Look up bans on the
+            # specific email address first.
+            bans = config.db.store.find(
+                Ban, email=email, mailing_list=None)
+            if bans.count() > 0:
+                return True
+            # And now look for global pattern bans.
+            bans = config.db.store.find(Ban, mailing_list=None)
+            for ban in bans:
+                if (ban.email.startswith('^') and
+                    re.match(ban.email, email, re.IGNORECASE) is not None):
+                    return True
+        return False
diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py
index cdebd5ca6..488b6da3d 100644
--- a/src/mailman/model/mailinglist.py
+++ b/src/mailman/model/mailinglist.py
@@ -108,8 +108,7 @@ class MailingList(Model):
     filter_content = Bool()
     collapse_alternatives = Bool()
     convert_html_to_plaintext = Bool()
-    # Bounces and bans.
-    ban_list = Pickle() # XXX
+    # Bounces.
     bounce_info_stale_after = TimeDelta() # XXX
     bounce_matching_headers = Unicode() # XXX
     bounce_notify_owner_on_disable = Bool() # XXX
diff --git a/src/mailman/styles/default.py b/src/mailman/styles/default.py
index f35b2519a..4f2b3d7d5 100644
--- a/src/mailman/styles/default.py
+++ b/src/mailman/styles/default.py
@@ -131,8 +131,6 @@ ${listinfo_page}
         mlist.forward_auto_discards = True
         mlist.generic_nonmember_action = 1
         mlist.nonmember_rejection_notice = ''
-        # Ban lists
-        mlist.ban_list = []
         # Max autoresponses per day.  A mapping between addresses and a
         # 2-tuple of the date of the last autoresponse and the number of
         # autoresponses sent on that date.
diff --git a/src/mailman/tests/test_documentation.py b/src/mailman/tests/test_documentation.py
index 30bcdccc3..23cc189d0 100644
--- a/src/mailman/tests/test_documentation.py
+++ b/src/mailman/tests/test_documentation.py
@@ -253,7 +253,7 @@ def test_suite():
                 layer = getattr(sys.modules[package_path], 'layer', SMTPLayer)
             for filename in os.listdir(docsdir):
                 base, extension = os.path.splitext(filename)
-                if os.path.splitext(filename)[1] == '.txt':
+                if os.path.splitext(filename)[1] in ('.txt', '.rst'):
                     module_path = package_path + '.' + base
                     doctest_files[module_path] = (
                         os.path.join(docsdir, filename), layer)