15 files changed, 109 insertions, 71 deletions

diff --git a/src/mailman/app/moderator.py b/src/mailman/app/moderator.py
index 2a8c5f8c2..c82327184 100644
--- a/src/mailman/app/moderator.py
+++ b/src/mailman/app/moderator.py
@@ -134,7 +134,7 @@ def handle_message(mlist, id, action,
         # Start by getting the message from the message store.
         msg = message_store.get_message_by_id(message_id)
         # Delete moderation-specific entries from the message metadata.
-        for key in msgdata.keys():
+        for key in list(msgdata):
             if key.startswith('_mod_'):
                 del msgdata[key]
         # Add some metadata to indicate this message has now been approved.
diff --git a/src/mailman/bin/mailman.py b/src/mailman/bin/mailman.py
index 67f4d0910..f9352fac6 100644
--- a/src/mailman/bin/mailman.py
+++ b/src/mailman/bin/mailman.py
@@ -28,6 +28,7 @@ __all__ = [
 import os
 import argparse
 
+from functools import cmp_to_key
 from zope.interface.verify import verifyObject
 
 from mailman.core.i18n import _
@@ -77,9 +78,14 @@ def main():
             return -1
         elif other.name == 'help':
             return 1
+        elif command.name < other.name:
+            return -1
+        elif command.name == other.name:
+            return 0
         else:
-            return cmp(command.name, other.name)
-    subcommands.sort(cmp=sort_function)
+            assert command.name > other.name
+            return 1
+    subcommands.sort(key=cmp_to_key(sort_function))
     for command in subcommands:
         command_parser = subparser.add_parser(
             command.name, help=_(command.__doc__))
diff --git a/src/mailman/commands/cli_lists.py b/src/mailman/commands/cli_lists.py
index cf1bd2ead..9d857992c 100644
--- a/src/mailman/commands/cli_lists.py
+++ b/src/mailman/commands/cli_lists.py
@@ -135,12 +135,12 @@ class Create:
         self.parser = parser
         command_parser.add_argument(
             '--language',
-            type=unicode, metavar='CODE', help=_("""\
+            metavar='CODE', help=_("""\
             Set the list's preferred language to CODE, which must be a
             registered two letter language code."""))
         command_parser.add_argument(
             '-o', '--owner',
-            type=unicode, action='append', default=[],
+            action='append', default=[],
             dest='owners', metavar='OWNER', help=_("""\
             Specify a listowner email address.  If the address is not
             currently registered with Mailman, the address is registered and
diff --git a/src/mailman/rest/docs/addresses.rst b/src/mailman/rest/docs/addresses.rst
index fec0c194b..670a12ef5 100644
--- a/src/mailman/rest/docs/addresses.rst
+++ b/src/mailman/rest/docs/addresses.rst
@@ -64,13 +64,6 @@ But his address record can be accessed with the case-preserved version too.
     registered_on: 2005-08-01T07:49:23
     self_link: http://localhost:9001/3.0/addresses/bart.person@example.com
 
-A non-existent email address can't be retrieved.
-
-    >>> dump_json('http://localhost:9001/3.0/addresses/nobody@example.com')
-    Traceback (most recent call last):
-    ...
-    HTTPError: HTTP Error 404: 404 Not Found
-
 When an address has a real name associated with it, this is also available in
 the REST API.
 
diff --git a/src/mailman/rest/docs/basic.rst b/src/mailman/rest/docs/basic.rst
index aa0205874..576aacc2c 100644
--- a/src/mailman/rest/docs/basic.rst
+++ b/src/mailman/rest/docs/basic.rst
@@ -24,13 +24,10 @@ Credentials
 When the `Authorization` header contains the proper credentials, the request
 succeeds.
 
-    >>> from base64 import b64encode
     >>> from httplib2 import Http
-    >>> auth = b64encode('{0}:{1}'.format(config.webservice.admin_user,
-    ...                                   config.webservice.admin_pass))
     >>> headers = {
     ...     'Content-Type': 'application/x-www-form-urlencode',
-    ...     'Authorization': 'Basic ' + auth,
+    ...     'Authorization': 'Basic cmVzdGFkbWluOnJlc3RwYXNz',
     ...     }
     >>> url = 'http://localhost:9001/3.0/system'
     >>> response, content = Http().request(url, 'GET', None, headers)
diff --git a/src/mailman/rest/docs/domains.rst b/src/mailman/rest/docs/domains.rst
index b28326f73..a78dacd85 100644
--- a/src/mailman/rest/docs/domains.rst
+++ b/src/mailman/rest/docs/domains.rst
@@ -228,13 +228,5 @@ Domains can also be deleted via the API.
     server: ...
     status: 204
 
-It is an error to delete a domain twice.
-
-    >>> dump_json('http://localhost:9001/3.0/domains/lists.example.com',
-    ...           method='DELETE')
-    Traceback (most recent call last):
-    ...
-    HTTPError: HTTP Error 404: 404 Not Found
-
 
 .. _Domains: ../../model/docs/domains.html
diff --git a/src/mailman/rest/docs/helpers.rst b/src/mailman/rest/docs/helpers.rst
index 2dd65bbb8..5614e6544 100644
--- a/src/mailman/rest/docs/helpers.rst
+++ b/src/mailman/rest/docs/helpers.rst
@@ -45,7 +45,7 @@ gets modified to contain the etag under the ``http_etag`` key.
     >>> resource = dict(geddy='bass', alex='guitar', neil='drums')
     >>> json_data = etag(resource)
     >>> print(resource['http_etag'])
-    "96e036d66248cab746b7d97047e08896fcfb2493"
+    "6929ecfbda2282980a4818fb75f82e812077f77a"
 
 For convenience, the etag function also returns the JSON representation of the
 dictionary after tagging, since that's almost always what you want.
@@ -58,7 +58,7 @@ dictionary after tagging, since that's almost always what you want.
     >>> dump_msgdata(data)
     alex     : guitar
     geddy    : bass
-    http_etag: "96e036d66248cab746b7d97047e08896fcfb2493"
+    http_etag: "6929ecfbda2282980a4818fb75f82e812077f77a"
     neil     : drums
 
 
@@ -82,7 +82,7 @@ On valid input, the validator can be used as a ``**keyword`` argument.
     >>> def print_request(one, two, three):
     ...     print(repr(one), repr(two), repr(three))
     >>> print_request(**validator(FakeRequest))
-    1 u'two' True
+    1 'two' True
 
 On invalid input, an exception is raised.
 
@@ -129,15 +129,15 @@ However, if optional keys are missing, it's okay.
     >>> def print_request(one, two, three, four=None, five=None):
     ...     print(repr(one), repr(two), repr(three), repr(four), repr(five))
     >>> print_request(**validator(FakeRequest))
-    1 u'two' True 4 5
+    1 'two' True 4 5
 
     >>> del FakeRequest.params['four']
     >>> print_request(**validator(FakeRequest))
-    1 u'two' True None 5
+    1 'two' True None 5
 
     >>> del FakeRequest.params['five']
     >>> print_request(**validator(FakeRequest))
-    1 u'two' True None None
+    1 'two' True None None
 
 But if the optional values are present, they must of course also be valid.
 
diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst
index 30e69d9f5..b0b884d51 100644
--- a/src/mailman/rest/docs/membership.rst
+++ b/src/mailman/rest/docs/membership.rst
@@ -572,7 +572,7 @@ Elly is now a known user, and a member of the mailing list.
     <User "Elly Person" (...) at ...>
 
     >>> set(member.list_id for member in elly.memberships.members)
-    set([u'ant.example.com'])
+    {'ant.example.com'}
 
     >>> dump_json('http://localhost:9001/3.0/members')
     entry 0:
@@ -674,7 +674,7 @@ so she leaves from the mailing list.
 Elly is no longer a member of the mailing list.
 
     >>> set(member.mailing_list for member in elly.memberships.members)
-    set([])
+    set()
 
 
 Digest delivery
diff --git a/src/mailman/rest/docs/moderation.rst b/src/mailman/rest/docs/moderation.rst
index 6e2dbb43c..6aec921f0 100644
--- a/src/mailman/rest/docs/moderation.rst
+++ b/src/mailman/rest/docs/moderation.rst
@@ -141,13 +141,6 @@ The held message can be discarded.
     server: ...
     status: 204
 
-After which, the message is gone from the moderation queue.
-
-    >>> dump_json(url(request_id))
-    Traceback (most recent call last):
-    ...
-    HTTPError: HTTP Error 404: 404 Not Found
-
 Messages can also be accepted via the REST API.  Let's hold a new message for
 moderation.
 ::
diff --git a/src/mailman/rest/docs/preferences.rst b/src/mailman/rest/docs/preferences.rst
index b9332c954..172a9bedd 100644
--- a/src/mailman/rest/docs/preferences.rst
+++ b/src/mailman/rest/docs/preferences.rst
@@ -162,7 +162,7 @@ deleted.
     >>> dump_json('http://localhost:9001/3.0/addresses/anne@example.com'
     ...           '/preferences')
     acknowledge_posts: True
-    http_etag: "1ff07b0367bede79ade27d217e12df3915aaee2b"
+    http_etag: "..."
     preferred_language: ja
     self_link: http://localhost:9001/3.0/addresses/anne@example.com/preferences
 
diff --git a/src/mailman/rest/docs/users.rst b/src/mailman/rest/docs/users.rst
index 04533f578..dcebba3e6 100644
--- a/src/mailman/rest/docs/users.rst
+++ b/src/mailman/rest/docs/users.rst
@@ -277,27 +277,6 @@ Users can also be deleted via the API.
     server: ...
     status: 204
 
-Cris's resource cannot be retrieved either by email address...
-
-    >>> dump_json('http://localhost:9001/3.0/users/cris@example.com')
-    Traceback (most recent call last):
-    ...
-    HTTPError: HTTP Error 404: 404 Not Found
-
-...or user id.
-
-    >>> dump_json('http://localhost:9001/3.0/users/3')
-    Traceback (most recent call last):
-    ...
-    HTTPError: HTTP Error 404: 404 Not Found
-
-Cris's address records no longer exist either.
-
-    >>> dump_json('http://localhost:9001/3.0/addresses/cris@example.com')
-    Traceback (most recent call last):
-    ...
-    HTTPError: HTTP Error 404: 404 Not Found
-
 
 User addresses
 ==============
@@ -416,12 +395,3 @@ This time, Elly successfully logs into Mailman.
     date: ...
     server: ...
     status: 204
-
-But this time, she is unsuccessful.
-
-    >>> dump_json('http://localhost:9001/3.0/users/5/login', {
-    ...           'cleartext_password': 'not-the-password',
-    ...           }, method='POST')
-    Traceback (most recent call last):
-    ...
-    HTTPError: HTTP Error 403: 403 Forbidden
diff --git a/src/mailman/rest/tests/test_addresses.py b/src/mailman/rest/tests/test_addresses.py
index 4d427df9f..ea850da9b 100644
--- a/src/mailman/rest/tests/test_addresses.py
+++ b/src/mailman/rest/tests/test_addresses.py
@@ -52,6 +52,12 @@ class TestAddresses(unittest.TestCase):
         self.assertEqual(json['start'], 0)
         self.assertEqual(json['total_size'], 0)
 
+    def test_missing_address(self):
+        # An address that isn't registered yet cannot be retrieved.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/addresses/nobody@example.com')
+        self.assertEqual(cm.exception.code, 404)
+
     def test_membership_of_missing_address(self):
         # Try to get the memberships of a missing address.
         with self.assertRaises(HTTPError) as cm:
diff --git a/src/mailman/rest/tests/test_domains.py b/src/mailman/rest/tests/test_domains.py
index 48f9c4fe3..cda9a9b89 100644
--- a/src/mailman/rest/tests/test_domains.py
+++ b/src/mailman/rest/tests/test_domains.py
@@ -64,7 +64,7 @@ class TestDomains(unittest.TestCase):
         content, response = call_api(
             'http://localhost:9001/3.0/domains/example.com', method='DELETE')
         self.assertEqual(response.status, 204)
-        self.assertEqual(getUtility(IListManager).get('ant@example.com'), None)
+        self.assertIsNone(getUtility(IListManager).get('ant@example.com'))
 
     def test_missing_domain(self):
         # You get a 404 if you try to access a nonexisting domain.
@@ -79,3 +79,14 @@ class TestDomains(unittest.TestCase):
             call_api(
                 'http://localhost:9001/3.0/domains/does-not-exist.com/lists')
         self.assertEqual(cm.exception.code, 404)
+
+    def test_double_delete(self):
+        # You cannot delete a domain twice.
+        content, response = call_api(
+            'http://localhost:9001/3.0/domains/example.com',
+            method='DELETE')
+        self.assertEqual(response.status, 204)
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/domains/example.com',
+                     method='DELETE')
+        self.assertEqual(cm.exception.code, 404)
diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py
index c3daf46de..207123168 100644
--- a/src/mailman/rest/tests/test_moderation.py
+++ b/src/mailman/rest/tests/test_moderation.py
@@ -125,3 +125,16 @@ Something else.
         self.assertEqual(cm.exception.code, 400)
         self.assertEqual(cm.exception.msg,
                          b'Cannot convert parameters: action')
+
+    def test_discard(self):
+        # Discarding a message removes it from the moderation queue.
+        with transaction():
+            held_id = hold_message(self._mlist, self._msg)
+        url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{}'.format(
+            held_id)
+        content, response = call_api(url, dict(action='discard'))
+        self.assertEqual(response.status, 204)
+        # Now it's gone.
+        with self.assertRaises(HTTPError) as cm:
+            call_api(url, dict(action='discard'))
+        self.assertEqual(cm.exception.code, 404)
diff --git a/src/mailman/rest/tests/test_users.py b/src/mailman/rest/tests/test_users.py
index a130b1cc9..d4d49889d 100644
--- a/src/mailman/rest/tests/test_users.py
+++ b/src/mailman/rest/tests/test_users.py
@@ -107,6 +107,48 @@ class TestUsers(unittest.TestCase):
                      method='DELETE')
         self.assertEqual(cm.exception.code, 404)
 
+    def test_delete_user_twice(self):
+        # You cannot DELETE a user twice, either by address or user id.
+        with transaction():
+            anne = getUtility(IUserManager).create_user(
+                'anne@example.com', 'Anne Person')
+            user_id = anne.user_id
+        content, response = call_api(
+            'http://localhost:9001/3.0/users/anne@example.com',
+            method='DELETE')
+        self.assertEqual(response.status, 204)
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/users/anne@example.com',
+                     method='DELETE')
+        self.assertEqual(cm.exception.code, 404)
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/users/{}'.format(user_id),
+                     method='DELETE')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_get_after_delete(self):
+        # You cannot GET a user record after deleting them.
+        with transaction():
+            anne = getUtility(IUserManager).create_user(
+                'anne@example.com', 'Anne Person')
+            user_id = anne.user_id
+        # You can still GET the user record.
+        content, response = call_api(
+            'http://localhost:9001/3.0/users/anne@example.com')
+        self.assertEqual(response.status, 200)
+        # Delete the user.
+        content, response = call_api(
+            'http://localhost:9001/3.0/users/anne@example.com',
+            method='DELETE')
+        self.assertEqual(response.status, 204)
+        # The user record can no longer be retrieved.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/users/anne@example.com')
+        self.assertEqual(cm.exception.code, 404)
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/users/{}'.format(user_id))
+        self.assertEqual(cm.exception.code, 404)
+
     def test_existing_user_error(self):
         # Creating a user twice results in an error.
         call_api('http://localhost:9001/3.0/users', {
@@ -250,6 +292,21 @@ class TestLogin(unittest.TestCase):
                 'anne@example.com', 'Anne Person')
             self.anne.password = config.password_context.encrypt('abc123')
 
+    def test_login_with_cleartext_password(self):
+        # A user can log in with the correct clear text password.
+        content, response = call_api(
+            'http://localhost:9001/3.0/users/anne@example.com/login', {
+                'cleartext_password': 'abc123',
+                }, method='POST')
+        self.assertEqual(response.status, 204)
+        # But the user cannot log in with an incorrect password.
+        with self.assertRaises(HTTPError) as cm:
+            call_api(
+                'http://localhost:9001/3.0/users/anne@example.com/login', {
+                    'cleartext_password': 'not-the-password',
+                    }, method='POST')
+        self.assertEqual(cm.exception.code, 403)
+
     def test_wrong_parameter(self):
         # A bad request because it is mistyped the required attribute.
         with self.assertRaises(HTTPError) as cm: