11 files changed, 99 insertions, 31 deletions

diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py
index 56df18269..aaf7f05df 100644
--- a/src/mailman/app/membership.py
+++ b/src/mailman/app/membership.py
@@ -30,7 +30,6 @@ from email.utils import formataddr
 from zope.component import getUtility
 
 from mailman.app.notifications import send_goodbye_message
-from mailman.config import config
 from mailman.core.i18n import _
 from mailman.email.message import OwnerNotification
 from mailman.interfaces.address import IEmailValidator
@@ -40,7 +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 lookup_scheme, make_secret
+from mailman.utilities.passwords import encrypt_password
 
 
 
@@ -98,9 +97,7 @@ def add_member(mlist, email, realname, password, delivery_mode, language):
             user.link(address)
         # Encrypt the password using the currently selected scheme.  The
         # scheme is recorded in the hashed password string.
-        user.password = make_secret(
-            password,
-            lookup_scheme(config.passwords.password_scheme))
+        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/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/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/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 2e765ea3e..c7258dd08 100644
--- a/src/mailman/testing/layers.py
+++ b/src/mailman/testing/layers.py
@@ -104,6 +104,8 @@ class ConfigLayer(MockAndMonkeyLayer):
         test_config = dedent("""
         [mailman]
         layout: testing
+        [passwords]
+        password_scheme: cleartext
         [paths.testing]
         var_dir: %s
         """ % cls.var_dir)
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():