17 files changed, 138 insertions, 40 deletions

diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py
index fcbedc2f5..aaf7f05df 100644
--- a/src/mailman/app/membership.py
+++ b/src/mailman/app/membership.py
@@ -39,6 +39,7 @@ from mailman.interfaces.member import (
     NotAMemberError)
 from mailman.interfaces.usermanager import IUserManager
 from mailman.utilities.i18n import make
+from mailman.utilities.passwords import encrypt_password
 
 
 
@@ -94,9 +95,9 @@ def add_member(mlist, email, realname, password, delivery_mode, language):
             user = user_manager.create_user()
             user.real_name = (realname if realname else address.real_name)
             user.link(address)
-        # Since created the user, then the member, and set preferences on the
-        # appropriate object.
-        user.password = password
+        # Encrypt the password using the currently selected scheme.  The
+        # scheme is recorded in the hashed password string.
+        user.password = encrypt_password(password)
         user.preferences.preferred_language = language
         member = address.subscribe(mlist, MemberRole.member)
         member.preferences.delivery_mode = delivery_mode
diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py
index b0e1bae5d..2b69c7f39 100644
--- a/src/mailman/app/tests/test_membership.py
+++ b/src/mailman/app/tests/test_membership.py
@@ -31,6 +31,7 @@ from zope.component import getUtility
 
 from mailman.app.lifecycle import create_list
 from mailman.app.membership import add_member
+from mailman.config import config
 from mailman.core.constants import system_preferences
 from mailman.interfaces.bans import IBanManager
 from mailman.interfaces.member import DeliveryMode, MembershipIsBannedError
@@ -125,7 +126,34 @@ class AddMemberTest(unittest.TestCase):
 
 
 
+class AddMemberPasswordTest(unittest.TestCase):
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+        # The default ssha scheme introduces a random salt, which is
+        # inappropriate for unit tests.
+        config.push('password scheme', """
+        [passwords]
+        password_scheme: sha
+        """)
+
+    def tearDown(self):
+        config.pop('password scheme')
+        reset_the_world()
+
+    def test_add_member_password(self):
+        # Test that the password stored with the new user is encrypted.
+        member = add_member(self._mlist, 'anne@example.com',
+                            'Anne Person', 'abc', DeliveryMode.regular,
+                            system_preferences.preferred_language)
+        self.assertEqual(
+            member.user.password, '{SHA}qZk-NkcGgWq6PiVxeFDCbJzQ2J0=')
+
+
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(AddMemberTest))
+    suite.addTest(unittest.makeSuite(AddMemberPasswordTest))
     return suite
diff --git a/src/mailman/commands/cli_members.py b/src/mailman/commands/cli_members.py
index cd7fcfbf1..96469c0f1 100644
--- a/src/mailman/commands/cli_members.py
+++ b/src/mailman/commands/cli_members.py
@@ -40,6 +40,7 @@ from mailman.interfaces.command import ICLISubCommand
 from mailman.interfaces.listmanager import IListManager
 from mailman.interfaces.member import (
     AlreadySubscribedError, DeliveryMode, DeliveryStatus)
+from mailman.utilities.passwords import make_user_friendly_password
 
 
 
@@ -196,8 +197,10 @@ class Members:
                 real_name, email = parseaddr(line)
                 real_name = real_name.decode(fp.encoding)
                 email = email.decode(fp.encoding)
+                # Give the user a default, user-friendly password.
+                password = make_user_friendly_password()
                 try:
-                    add_member(mlist, email, real_name, None,
+                    add_member(mlist, email, real_name, password,
                                DeliveryMode.regular,
                                mlist.preferred_language.code)
                 except AlreadySubscribedError:
diff --git a/src/mailman/commands/docs/info.txt b/src/mailman/commands/docs/info.txt
index 12fce3223..9658e93e6 100644
--- a/src/mailman/commands/docs/info.txt
+++ b/src/mailman/commands/docs/info.txt
@@ -60,7 +60,6 @@ The File System Hierarchy layout is the same every by definition.
     ...
     File system paths:
         BIN_DIR                  = /sbin
-        CREATOR_PW_FILE          = /var/lib/mailman/data/creator.pw
         DATA_DIR                 = /var/lib/mailman/data
         ETC_DIR                  = /etc
         EXT_DIR                  = /etc/mailman.d
@@ -73,7 +72,6 @@ The File System Hierarchy layout is the same every by definition.
         PRIVATE_ARCHIVE_FILE_DIR = /var/lib/mailman/archives/private
         PUBLIC_ARCHIVE_FILE_DIR  = /var/lib/mailman/archives/public
         QUEUE_DIR                = /var/spool/mailman
-        SITE_PW_FILE             = /var/lib/mailman/data/adm.pw
         TEMPLATE_DIR             = .../mailman/templates
         VAR_DIR                  = /var/lib/mailman
 
diff --git a/src/mailman/config/config.py b/src/mailman/config/config.py
index 636b9ef9e..9c210b6a2 100644
--- a/src/mailman/config/config.py
+++ b/src/mailman/config/config.py
@@ -183,10 +183,8 @@ class Configuration:
                 if category.template_dir == ':source:'
                 else category.template_dir),
             # Files.
-            creator_pw_file         = category.creator_pw_file,
             lock_file               = category.lock_file,
             pid_file                = category.pid_file,
-            site_pw_file            = category.site_pw_file,
             )
         # Now, perform substitutions recursively until there are no more
         # variables with $-vars in them, or until substitutions are not
diff --git a/src/mailman/config/mailman.cfg b/src/mailman/config/mailman.cfg
index f6811d7c9..d7bc0fded 100644
--- a/src/mailman/config/mailman.cfg
+++ b/src/mailman/config/mailman.cfg
@@ -38,8 +38,6 @@ lock_dir: /var/lock/mailman
 etc_dir: /etc
 ext_dir: /etc/mailman.d
 pid_file: /var/run/mailman/master-qrunner.pid
-creator_pw_file: $data_dir/creator.pw
-site_pw_file: $data_dir/adm.pw
 
 [language.en]
 
diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index f67b21637..030e5fc2c 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -115,10 +115,6 @@ template_dir: :source:
 #
 # This is where PID file for the master queue runner is stored.
 pid_file: $var_dir/master-qrunner.pid
-# The site administrators password [obsolete].
-site_pw_file: $var_dir/adm.pw
-# The site list creator's password [obsolete].
-creator_pw_file: $var_dir/creator.pw
 # Lock file.
 lock_file: $lock_dir/master-qrunner.lck
 
@@ -140,6 +136,11 @@ testing: no
 
 
 [passwords]
+# The default scheme to use to encrypt new passwords.  Existing passwords
+# include the scheme that was used to encrypt them, so it's okay to change
+# this after users have been added.
+password_scheme: ssha
+
 # When Mailman generates them, this is the default length of passwords.
 password_length: 8
 
diff --git a/src/mailman/database/mailman.sql b/src/mailman/database/mailman.sql
index 7d67dea05..7c09fb79f 100644
--- a/src/mailman/database/mailman.sql
+++ b/src/mailman/database/mailman.sql
@@ -251,7 +251,7 @@ CREATE TABLE preferences (
 CREATE TABLE user (
         id INTEGER NOT NULL,
         real_name TEXT,
-        password TEXT,
+        password BINARY,
         _user_id TEXT,
         _created_on TIMESTAMP,
         preferences_id INTEGER,
diff --git a/src/mailman/model/docs/requests.txt b/src/mailman/model/docs/requests.txt
index 94c81e1dc..2ff173422 100644
--- a/src/mailman/model/docs/requests.txt
+++ b/src/mailman/model/docs/requests.txt
@@ -692,15 +692,9 @@ Frank Person is now a member of the mailing list.
     <Language [en] English (USA)>
     >>> print member.delivery_mode
     DeliveryMode.regular
-
-    >>> from mailman.interfaces.usermanager import IUserManager
-    >>> from zope.component import getUtility
-    >>> user_manager = getUtility(IUserManager)
-
-    >>> user = user_manager.get_user(member.address.email)
-    >>> print user.real_name
+    >>> print member.user.real_name
     Frank Person
-    >>> print user.password
+    >>> print member.user.password
     {NONE}abcxyz
 
 
@@ -713,6 +707,11 @@ unsubscription holds can send the list's moderators an immediate
 notification.
 ::
 
+
+    >>> from mailman.interfaces.usermanager import IUserManager
+    >>> from zope.component import getUtility
+    >>> user_manager = getUtility(IUserManager)
+
     >>> mlist.admin_immed_notify = False
     >>> from mailman.interfaces.member import MemberRole
     >>> user_1 = user_manager.create_user('gperson@example.com')
diff --git a/src/mailman/model/docs/usermanager.txt b/src/mailman/model/docs/usermanager.txt
index 8304e659c..e427eb63a 100644
--- a/src/mailman/model/docs/usermanager.txt
+++ b/src/mailman/model/docs/usermanager.txt
@@ -44,7 +44,7 @@ A user can be assigned a real name.
 
 A user can be assigned a password.
 
-    >>> user.password = 'secret'
+    >>> user.password = b'secret'
     >>> dump_list(user.password for user in user_manager.users)
     secret
 
diff --git a/src/mailman/model/docs/users.txt b/src/mailman/model/docs/users.txt
index 31cc58918..c8244c506 100644
--- a/src/mailman/model/docs/users.txt
+++ b/src/mailman/model/docs/users.txt
@@ -19,7 +19,7 @@ User data
 Users may have a real name and a password.
 
     >>> user_1 = user_manager.create_user()
-    >>> user_1.password = 'my password'
+    >>> user_1.password = b'my password'
     >>> user_1.real_name = 'Zoe Person'
     >>> dump_list(user.real_name for user in user_manager.users)
     Zoe Person
@@ -29,7 +29,7 @@ Users may have a real name and a password.
 The password and real name can be changed at any time.
 
     >>> user_1.real_name = 'Zoe X. Person'
-    >>> user_1.password = 'another password'
+    >>> user_1.password = b'another password'
     >>> dump_list(user.real_name for user in user_manager.users)
     Zoe X. Person
     >>> dump_list(user.password for user in user_manager.users)
diff --git a/src/mailman/model/user.py b/src/mailman/model/user.py
index 39f4fa240..f0048c5f4 100644
--- a/src/mailman/model/user.py
+++ b/src/mailman/model/user.py
@@ -24,7 +24,8 @@ __all__ = [
     'User',
     ]
 
-from storm.locals import DateTime, Int, Reference, ReferenceSet, Unicode
+from storm.locals import (
+    DateTime, Int, RawStr, Reference, ReferenceSet, Unicode)
 from zope.interface import implements
 
 from mailman.config import config
@@ -47,7 +48,7 @@ class User(Model):
 
     id = Int(primary=True)
     real_name = Unicode()
-    password = Unicode()
+    password = RawStr()
     _user_id = Unicode()
     _created_on = DateTime()
 
diff --git a/src/mailman/mta/docs/decorating.txt b/src/mailman/mta/docs/decorating.txt
index 4edac481f..0bc9649c8 100644
--- a/src/mailman/mta/docs/decorating.txt
+++ b/src/mailman/mta/docs/decorating.txt
@@ -68,17 +68,17 @@ list.
     >>> user_manager = getUtility(IUserManager)
 
     >>> anne = user_manager.create_user('aperson@example.com', 'Anne Person')
-    >>> anne.password = 'AAA'
+    >>> anne.password = b'AAA'
     >>> list(anne.addresses)[0].subscribe(mlist, MemberRole.member)
     <Member: Anne Person <aperson@example.com> ...
 
     >>> bart = user_manager.create_user('bperson@example.com', 'Bart Person')
-    >>> bart.password = 'BBB'
+    >>> bart.password = b'BBB'
     >>> list(bart.addresses)[0].subscribe(mlist, MemberRole.member)
     <Member: Bart Person <bperson@example.com> ...
 
     >>> cris = user_manager.create_user('cperson@example.com', 'Cris Person')
-    >>> cris.password = 'CCC'
+    >>> cris.password = b'CCC'
     >>> list(cris.addresses)[0].subscribe(mlist, MemberRole.member)
     <Member: Cris Person <cperson@example.com> ...
 
diff --git a/src/mailman/rest/adapters.py b/src/mailman/rest/adapters.py
index 30ce99f44..5cbb89bc1 100644
--- a/src/mailman/rest/adapters.py
+++ b/src/mailman/rest/adapters.py
@@ -36,6 +36,7 @@ from mailman.interfaces.address import InvalidEmailAddressError
 from mailman.interfaces.listmanager import IListManager, NoSuchListError
 from mailman.interfaces.member import DeliveryMode
 from mailman.interfaces.membership import ISubscriptionService
+from mailman.utilities.passwords import make_user_friendly_password
 
 
 
@@ -84,15 +85,12 @@ class SubscriptionService:
                 raise InvalidEmailAddressError(address)
         # Because we want to keep the REST API simple, there is no password or
         # language given to us.  We'll use the system's default language for
-        # the user's default language.  We'll set the password to None.  XXX
-        # Is that a good idea?  Maybe we should set it to something else,
-        # except that once we encode the password (as we must do to avoid
-        # cleartext passwords in the database) we'll never be able to retrieve
-        # it.
-        #
+        # the user's default language.  We'll set the password to a system
+        # default.  This will have to get reset since it can't be retrieved.
         # Note that none of these are used unless the address is completely
         # new to us.
-        return add_member(mlist, address, real_name, None, mode,
+        password = make_user_friendly_password()
+        return add_member(mlist, address, real_name, password, mode,
                           system_preferences.preferred_language)
 
     def leave(self, fqdn_listname, address):
diff --git a/src/mailman/testing/layers.py b/src/mailman/testing/layers.py
index ca417fc2c..29ab7169a 100644
--- a/src/mailman/testing/layers.py
+++ b/src/mailman/testing/layers.py
@@ -105,6 +105,8 @@ class ConfigLayer(MockAndMonkeyLayer):
         test_config = dedent("""
         [mailman]
         layout: testing
+        [passwords]
+        password_scheme: cleartext
         [paths.testing]
         var_dir: %s
         [devmode]
diff --git a/src/mailman/utilities/passwords.py b/src/mailman/utilities/passwords.py
index 0de4255da..896872436 100644
--- a/src/mailman/utilities/passwords.py
+++ b/src/mailman/utilities/passwords.py
@@ -26,6 +26,7 @@ __metaclass__ = type
 __all__ = [
     'Schemes',
     'check_response',
+    'encrypt_password',
     'make_secret',
     'make_user_friendly_password',
     ]
@@ -50,6 +51,7 @@ from mailman.core import errors
 SALT_LENGTH = 20 # bytes
 ITERATIONS  = 2000
 EMPTYSTRING = ''
+SCHEME_RE = r'{(?P<scheme>[^}]+?)}(?P<rest>.*)'
 
 
 
@@ -294,8 +296,7 @@ def check_response(challenge, response):
     :return: True if the response matches the challenge.
     :rtype: bool
     """
-    mo = re.match(r'{(?P<scheme>[^}]+?)}(?P<rest>.*)',
-                  challenge, re.IGNORECASE)
+    mo = re.match(SCHEME_RE, challenge, re.IGNORECASE)
     if not mo:
         return False
     # See above for why we convert here.  However because we should have
@@ -323,6 +324,42 @@ def lookup_scheme(scheme_name):
     return _SCHEMES_BY_TAG.get(scheme_name.lower())
 
 
+def encrypt_password(password, scheme=None):
+    """Return an encrypted password.
+
+    If the given password is already encrypted (i.e. it has a scheme prefix),
+    then the password is return unchanged.  Otherwise, it is encrypted with
+    the given scheme or the default scheme.
+
+    :param password: The plain text or encrypted password.
+    :type password: string
+    :param scheme: The scheme enum to use for encryption.  If not given, the
+        system default scheme is used.  This can be a `Schemes` enum item, or
+        the scheme name as a string.
+    :type scheme: `Schemes` enum, or string.
+    :return: The encrypted password.
+    :rtype: bytes
+    """
+    if not isinstance(password, (bytes, unicode)):
+        raise ValueError('Got {0}, expected unicode or bytes'.format(
+                         type(password)))
+    if re.match(SCHEME_RE, password, re.IGNORECASE):
+        # Just ensure we're getting bytes back.
+        if isinstance(password, unicode):
+            return password.encode('us-ascii')
+        assert isinstance(password, bytes), 'Expected bytes'
+        return password
+    if scheme is None:
+        password_scheme = lookup_scheme(config.passwords.password_scheme)
+    elif scheme in Schemes:
+        password_scheme = scheme
+    else:
+        password_scheme = lookup_scheme(scheme)
+    if password_scheme is None:
+        raise ValueError('Bad password scheme: {0}'.format(scheme))
+    return make_secret(password, password_scheme)
+
+
 
 # Password generation.
 
diff --git a/src/mailman/utilities/tests/test_passwords.py b/src/mailman/utilities/tests/test_passwords.py
index 7b6989779..c9b3d2e91 100644
--- a/src/mailman/utilities/tests/test_passwords.py
+++ b/src/mailman/utilities/tests/test_passwords.py
@@ -170,6 +170,40 @@ class TestPasswordGeneration(unittest.TestCase):
             self.assertTrue(vowel in 'aeiou', vowel)
             self.assertTrue(consonant not in 'aeiou', consonant)
 
+    def test_encrypt_password_plaintext_default_scheme(self):
+        # Test that a plain text password gets encrypted.
+        self.assertEqual(passwords.encrypt_password('abc'),
+                         '{CLEARTEXT}abc')
+
+    def test_encrypt_password_plaintext(self):
+        # Test that a plain text password gets encrypted with the given scheme.
+        scheme = passwords.Schemes.sha
+        self.assertEqual(passwords.encrypt_password('abc', scheme),
+                         '{SHA}qZk-NkcGgWq6PiVxeFDCbJzQ2J0=')
+
+    def test_encrypt_password_plaintext_by_scheme_name(self):
+        # Test that a plain text password gets encrypted with the given
+        # scheme, which is given by name.
+        self.assertEqual(passwords.encrypt_password('abc', 'cleartext'),
+                         '{CLEARTEXT}abc')
+
+    def test_encrypt_password_already_encrypted_default_scheme(self):
+        # Test that a password which is already encrypted is return unchanged.
+        self.assertEqual(passwords.encrypt_password('{SHA}abc'), '{SHA}abc')
+
+    def test_encrypt_password_already_encrypted(self):
+        # Test that a password which is already encrypted is return unchanged,
+        # ignoring any requested scheme.
+        scheme = passwords.Schemes.cleartext
+        self.assertEqual(passwords.encrypt_password('{SHA}abc', scheme),
+                         '{SHA}abc')
+
+    def test_encrypt_password_password_value_error(self):
+        self.assertRaises(ValueError, passwords.encrypt_password, 7)
+
+    def test_encrypt_password_scheme_value_error(self):
+        self.assertRaises(ValueError, passwords.encrypt_password, 'abc', 'foo')
+
 
 
 def test_suite():