diff options
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 4 | ||||
| -rw-r--r-- | src/mailman/rest/docs/users.rst | 42 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_users.py | 87 | ||||
| -rw-r--r-- | src/mailman/rest/users.py | 22 | ||||
| -rw-r--r-- | src/mailman/testing/passlib.cfg | 8 | ||||
| -rw-r--r-- | src/mailman/utilities/passwords.py | 4 |
6 files changed, 164 insertions, 3 deletions
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index c0a94ab0d..aad8f37ff 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -53,6 +53,10 @@ REST address does not change its `.verified_on` date. (LP: #1054730) * Deleting a user through the REST API also deletes all the user's linked addresses and memberships. (LP: #1074374) + * A user's password can be verified by POSTing to .../user/<id>/login. The + data must contain a single parameter `cleartext_password` and if this + matches, a 204 (No Content) will be returned, otherwise a 403 (Forbidden) + is returned. (LP: #1065447) Configuration ------------- diff --git a/src/mailman/rest/docs/users.rst b/src/mailman/rest/docs/users.rst index 8ec455f91..36ec28efc 100644 --- a/src/mailman/rest/docs/users.rst +++ b/src/mailman/rest/docs/users.rst @@ -352,3 +352,45 @@ addresses can be used to look up Fred's user record. http_etag: "..." self_link: http://localhost:9001/3.0/users/6 user_id: 6 + + +Verifying passwords +=================== + +A user's password is stored internally in hashed form. Logging in a user is +the process of verifying a provided clear text password against the hashed +internal password. + +When Elly was added as a user, she provided a password in the clear. Now the +password is hashed and getting her user record returns the hashed password. + + >>> dump_json('http://localhost:9001/3.0/users/5') + created_on: 2005-08-01T07:49:23 + display_name: Elly Person + http_etag: "..." + password: {plaintext}supersekrit + self_link: http://localhost:9001/3.0/users/5 + user_id: 5 + +Unless the client can run the hashing algorithm on the login text that Elly +provided, and do its own comparison, the client should let the REST API handle +password verification. + +This time, Elly successfully logs into Mailman. + + >>> dump_json('http://localhost:9001/3.0/users/5/login', { + ... 'cleartext_password': 'supersekrit', + ... }, method='POST') + content-length: 0 + 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_users.py b/src/mailman/rest/tests/test_users.py index 805baf67e..ae9b8130e 100644 --- a/src/mailman/rest/tests/test_users.py +++ b/src/mailman/rest/tests/test_users.py @@ -21,20 +21,23 @@ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ - 'TestUsers', 'TestLP1074374', + 'TestLogin', + 'TestUsers', ] +import os import unittest from urllib2 import HTTPError from zope.component import getUtility from mailman.app.lifecycle import create_list +from mailman.config import config from mailman.database.transaction import transaction from mailman.interfaces.usermanager import IUserManager -from mailman.testing.helpers import call_api +from mailman.testing.helpers import call_api, configuration from mailman.testing.layers import RESTLayer @@ -131,6 +134,22 @@ class TestUsers(unittest.TestCase): call_api('http://localhost:9001/3.0/users/z@example.net/addresses') self.assertEqual(cm.exception.code, 404) + def test_login_missing_user_by_id(self): + # Verify a password for a non-existing user, by id. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/users/99/login', { + 'cleartext_password': 'wrong', + }) + self.assertEqual(cm.exception.code, 404) + + def test_login_missing_user_by_address(self): + # Verify a password for a non-existing user, by address. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/users/z@example.org/login', { + 'cleartext_password': 'wrong', + }) + self.assertEqual(cm.exception.code, 404) + class TestLP1074374(unittest.TestCase): @@ -214,3 +233,67 @@ class TestLP1074374(unittest.TestCase): self.assertEqual(member['delivery_mode'], 'regular') self.assertEqual(member['list_id'], 'test.example.com') self.assertEqual(member['role'], 'member') + + + +class TestLogin(unittest.TestCase): + """Test user 'login' (really just password verification).""" + + layer = RESTLayer + + def setUp(self): + user_manager = getUtility(IUserManager) + with transaction(): + self.anne = user_manager.create_user( + 'anne@example.com', 'Anne Person') + self.anne.password = config.password_context.encrypt('abc123') + + def test_wrong_parameter(self): + # A bad request because it is mistyped the required attribute. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/users/1/login', { + 'hashed_password': 'bad hash', + }) + self.assertEqual(cm.exception.code, 400) + + def test_not_enough_parameters(self): + # A bad request because it is missing the required attribute. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/users/1/login', { + }) + self.assertEqual(cm.exception.code, 400) + + def test_too_many_parameters(self): + # A bad request because it has too many attributes. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/users/1/login', { + 'cleartext_password': 'abc123', + 'display_name': 'Annie Personhood', + }) + self.assertEqual(cm.exception.code, 400) + + def test_successful_login_updates_password(self): + # Passlib supports updating the hash when the hash algorithm changes. + # When a user logs in successfully, the password will be updated if + # necessary. + # + # Start by hashing Anne's password with a different hashing algorithm + # than the one that the REST runner uses by default during testing. + config_file = os.path.join(config.VAR_DIR, 'passlib-tmp.config') + with open(config_file, 'w') as fp: + print("""\ +[passlib] +schemes = hex_md5 +""", file=fp) + with configuration('passwords', configuration=config_file): + with transaction(): + self.anne.password = config.password_context.encrypt('abc123') + # Just ensure Anne's password is hashed correctly. + self.assertEqual(self.anne.password, + 'e99a18c428cb38d5f260853678922e03') + # Now, Anne logs in with a successful password. This should change it + # back to the plaintext hash. + call_api('http://localhost:9001/3.0/users/1/login', { + 'cleartext_password': 'abc123', + }) + self.assertEqual(self.anne.password, '{plaintext}abc123') diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index a7847f438..b67233f28 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -228,3 +228,25 @@ class AUser(_UserBase): except ValueError as error: return http.bad_request([], str(error)) return no_content() + + @resource.child('login') + def login(self, request, segments): + """Log the user in, sort of, by verifying a given password.""" + #import pdb; pdb.set_trace() + if self._user is None: + return http.not_found() + # We do not want to encrypt the plaintext password given in the POST + # data. That would hash the password, but we need to have the + # plaintext in order to pass into passlib. + validator = Validator(cleartext_password=GetterSetter(unicode)) + try: + values = validator(request) + except ValueError as error: + return http.bad_request([], str(error)) + is_valid, new_hash = config.password_context.verify( + values['cleartext_password'], self._user.password) + if is_valid: + if new_hash is not None: + self._user.password = new_hash + return no_content() + return http.forbidden() diff --git a/src/mailman/testing/passlib.cfg b/src/mailman/testing/passlib.cfg index 225ecd49b..2779ed89d 100644 --- a/src/mailman/testing/passlib.cfg +++ b/src/mailman/testing/passlib.cfg @@ -1,4 +1,10 @@ [passlib] # Use a predictable hashing algorithm with plain text and no salt. This is # *only* useful for debugging and unit testing. -schemes = roundup_plaintext +# +# We add the hex_md5 scheme for hash migration tests. The old hash will be +# hex_md5 (which is not salted and thus reproducible), but since this is +# deprecated here, it will get "ugpraded" to roundup_plaintext when +# successfully verified. +schemes = roundup_plaintext, hex_md5 +deprecated = hex_md5 diff --git a/src/mailman/utilities/passwords.py b/src/mailman/utilities/passwords.py index 44fdbc14f..9abc59402 100644 --- a/src/mailman/utilities/passwords.py +++ b/src/mailman/utilities/passwords.py @@ -63,6 +63,10 @@ class PasswordContext: :type password: :param hashed: The hash string to compare to. :type hashed: string + :return: 2-tuple where the first element is a flag indicating whether + the password verified or not, and the second value whether the + existing hash needs to be replaced (a str if so, else None). + :rtype: 2-tuple """ return self._context.verify_and_update(password, hashed) |