changeset 5918:5126fadbf24f

password related code and tests - clean up and refactor use user.DEFAULT_ALG constant instead of hardcoded default algorithm check auto-upgrade of stored hashes to DEFAULT_ALG at login time, remove the now redundant upgrade tests add a test for sha to DEFAULT_ALG (ssha) upgrade add a test for ssha auth reimplement ssha pw check in the same way as for other hash algorithms, so it is in no way specialcased. add another check whether hash upgrade is needed. cosmetic other changes
author Thomas Waldmann <tw AT waldmann-edv DOT de>
date Fri, 18 Jan 2013 01:29:28 +0100
parents c99f570e274a
children efd7c0be3339
files MoinMoin/_tests/test_user.py MoinMoin/user.py
diffstat 2 files changed, 87 insertions(+), 83 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/_tests/test_user.py	Thu Jan 17 22:39:00 2013 +0100
+++ b/MoinMoin/_tests/test_user.py	Fri Jan 18 01:29:28 2013 +0100
@@ -4,6 +4,7 @@
 
     @copyright: 2003-2004 by Juergen Hermann <jh@web.de>
                 2009 by ReimarBauer
+                2013 by MoinMoin:ThomasWaldmann
     @license: GNU GPL, see COPYING for details.
 """
 
@@ -12,6 +13,8 @@
 
 from MoinMoin import user, caching
 
+DEFAULT_ALG = user.DEFAULT_ALG
+
 
 class TestEncodePassword(object):
     """user: encode passwords tests"""
@@ -97,48 +100,99 @@
     def test_auth_with_apr1_stored_password(self):
         """
         Create user with {APR1} password and check that user can login.
+        Also check if auto-upgrade happens and is saved to disk.
         """
         # Create test user
         name = u'Test User'
+        password = '12345'
         # generated with "htpasswd -nbm blaze 12345"
-        password = '{APR1}$apr1$NG3VoiU5$PSpHT6tV0ZMKkSZ71E3qg.' # 12345
-        self.createUser(name, password, True)
+        pw_hash = '{APR1}$apr1$NG3VoiU5$PSpHT6tV0ZMKkSZ71E3qg.'
+        self.createUser(name, pw_hash, True)
 
         # Try to "login"
-        theuser = user.User(self.request, name=name, password='12345')
+        theuser = user.User(self.request, name=name, password=password)
         assert theuser.valid
+        # Check if the stored password was auto-upgraded on login and saved
+        theuser = user.User(self.request, name=name, password=password)
+        assert theuser.enc_password.startswith(DEFAULT_ALG)
 
     def test_auth_with_md5_stored_password(self):
         """
         Create user with {MD5} password and check that user can login.
+        Also check if auto-upgrade happens and is saved to disk.
         """
         # Create test user
         name = u'Test User'
-        password = '{MD5}$1$salt$etVYf53ma13QCiRbQOuRk/' # 12345
-        self.createUser(name, password, True)
+        password = '12345'
+        pw_hash = '{MD5}$1$salt$etVYf53ma13QCiRbQOuRk/'
+        self.createUser(name, pw_hash, True)
 
         # Try to "login"
-        theuser = user.User(self.request, name=name, password='12345')
+        theuser = user.User(self.request, name=name, password=password)
         assert theuser.valid
+        # Check if the stored password was auto-upgraded on login and saved
+        theuser = user.User(self.request, name=name, password=password)
+        assert theuser.enc_password.startswith(DEFAULT_ALG)
 
     def test_auth_with_des_stored_password(self):
         """
         Create user with {DES} password and check that user can login.
+        Also check if auto-upgrade happens and is saved to disk.
         """
         # Create test user
         name = u'Test User'
+        password = '12345'
         # generated with "htpasswd -nbd blaze 12345"
-        password = '{DES}gArsfn7O5Yqfo' # 12345
-        self.createUser(name, password, True)
+        pw_hash = '{DES}gArsfn7O5Yqfo'
+        self.createUser(name, pw_hash, True)
 
         try:
             import crypt
             # Try to "login"
-            theuser = user.User(self.request, name=name, password='12345')
+            theuser = user.User(self.request, name=name, password=password)
             assert theuser.valid
+            # Check if the stored password was auto-upgraded on login and saved
+            theuser = user.User(self.request, name=name, password=password)
+            assert theuser.enc_password.startswith(DEFAULT_ALG)
         except ImportError:
             py.test.skip("Platform does not provide crypt module!")
 
+    def test_auth_with_sha_stored_password(self):
+        """
+        Create user with {SHA} password and check that user can login.
+        Also check if auto-upgrade happens and is saved to disk.
+        """
+        # Create test user
+        name = u'Test User'
+        password = '12345'
+        pw_hash = '{SHA}jLIjfQZ5yojbZGTqxg2pY0VROWQ='
+        self.createUser(name, pw_hash, True)
+
+        # Try to "login"
+        theuser = user.User(self.request, name=name, password=password)
+        assert theuser.valid
+        # Check if the stored password was auto-upgraded on login and saved
+        theuser = user.User(self.request, name=name, password=password)
+        assert theuser.enc_password.startswith(DEFAULT_ALG)
+
+    def test_auth_with_ssha_stored_password(self):
+        """
+        Create user with {SSHA} password and check that user can login.
+        Also check if auto-upgrade happens and is saved to disk.
+        """
+        # Create test user
+        name = u'Test User'
+        password = '12345'
+        pw_hash = '{SSHA}dbeFtH5EGkOI1jgPADlGZgHWq072TIsKqWfHX7zZbUQa85Ze8774Rg=='
+        self.createUser(name, pw_hash, True)
+
+        # Try to "login"
+        theuser = user.User(self.request, name=name, password=password)
+        assert theuser.valid
+        # Check if the stored password was auto-upgraded on login and saved
+        theuser = user.User(self.request, name=name, password=password)
+        assert theuser.enc_password.startswith(DEFAULT_ALG)
+
     def testSubscriptionSubscribedPage(self):
         """ user: tests isSubscribedTo  """
         pagename = u'HelpMiscellaneous'
@@ -179,63 +233,6 @@
 
         assert not theUser.exists()
 
-    def test_upgrade_password_from_sha_to_ssha(self):
-        """
-        Create user with {SHA} password and check that logging in
-        upgrades to {SSHA}.
-        """
-        name = u'/no such user/'
-        password = '{SHA}jLIjfQZ5yojbZGTqxg2pY0VROWQ=' # 12345
-        self.createUser(name, password, True)
-
-        # User is not required to be valid
-        theuser = user.User(self.request, name=name, password='12345')
-        assert theuser.enc_password[:6] == '{SSHA}'
-
-    def test_upgrade_password_from_apr1_to_ssha(self):
-        """
-        Create user with {APR1} password and check that logging in
-        upgrades to {SSHA}.
-        """
-        # Create test user
-        name = u'Test User'
-        # generated with "htpasswd -nbm blaze 12345"
-        password = '{APR1}$apr1$NG3VoiU5$PSpHT6tV0ZMKkSZ71E3qg.' # 12345
-        self.createUser(name, password, True)
-
-        # User is not required to be valid
-        theuser = user.User(self.request, name=name, password='12345')
-        assert theuser.enc_password[:6] == '{SSHA}'
-
-    def test_upgrade_password_from_md5_to_ssha(self):
-        """
-        Create user with {MD5} password and check that logging in
-        upgrades to {SSHA}.
-        """
-        # Create test user
-        name = u'Test User'
-        password = '{MD5}$1$salt$etVYf53ma13QCiRbQOuRk/' # 12345
-        self.createUser(name, password, True)
-
-        # User is not required to be valid
-        theuser = user.User(self.request, name=name, password='12345')
-        assert theuser.enc_password[:6] == '{SSHA}'
-
-    def test_upgrade_password_from_des_to_ssha(self):
-        """
-        Create user with {DES} password and check that logging in
-        upgrades to {SSHA}.
-        """
-        # Create test user
-        name = u'Test User'
-        # generated with "htpasswd -nbd blaze 12345"
-        password = '{DES}gArsfn7O5Yqfo' # 12345
-        self.createUser(name, password, True)
-
-        # User is not required to be valid
-        theuser = user.User(self.request, name=name, password='12345')
-        assert theuser.enc_password[:6] == '{SSHA}'
-
     def test_for_email_attribute_by_name(self):
         """
         checks for no access to the email attribute by getting the user object from name
--- a/MoinMoin/user.py	Thu Jan 17 22:39:00 2013 +0100
+++ b/MoinMoin/user.py	Fri Jan 18 01:29:28 2013 +0100
@@ -35,6 +35,8 @@
 from MoinMoin.util import timefuncs, random_string
 from MoinMoin.wikiutil import url_quote_plus
 
+DEFAULT_ALG = '{SSHA}'  # default algorithm for pw hashing
+
 
 def getUserList(request):
     """ Get a list of all (numerical) user IDs.
@@ -148,13 +150,12 @@
 
 
 def encodePassword(pwd, salt=None):
-    """ Encode a cleartext password
+    """ Encode a cleartext password using the DEFAULT_ALG algorithm.
 
     @param pwd: the cleartext password, (unicode)
     @param salt: the salt for the password (string)
     @rtype: string
-    @return: the password in apache htpasswd compatible SHA-encoding,
-        or None
+    @return: the password hash in apache htpasswd compatible encoding,
     """
     pwd = pwd.encode('utf-8')
 
@@ -544,14 +545,21 @@
         if not password:
             return False, False
 
-        # Check and upgrade passwords from earlier MoinMoin versions and
-        # passwords imported from other wiki systems.
-        for method in ['{SHA}', '{APR1}', '{MD5}', '{DES}']:
+        # Check password and upgrade weak hashes to strong DEFAULT_ALG:
+        for method in ['{SSHA}', '{SHA}', '{APR1}', '{MD5}', '{DES}']:
             if epwd.startswith(method):
                 d = epwd[len(method):]
-                if method == '{SHA}':
+                if method == '{SSHA}':
+                    d = base64.decodestring(d)
+                    salt = d[20:]
+                    hash = hash_new('sha1', password.encode('utf-8'))
+                    hash.update(salt)
+                    enc = base64.encodestring(hash.digest() + salt).rstrip()
+
+                elif method == '{SHA}':
                     enc = base64.encodestring(
                         hash_new('sha1', password.encode('utf-8')).digest()).rstrip()
+
                 elif method == '{APR1}':
                     # d is of the form "$apr1$<salt>$<hash>"
                     salt = d.split('$')[2]
@@ -570,18 +578,17 @@
                     enc = crypt.crypt(password.encode('utf-8'), salt.encode('ascii'))
 
                 if safe_str_equal(epwd, method + enc):
-                    data['enc_password'] = encodePassword(password) # upgrade to SSHA
-                    return True, True
+                    # hashes match, password is correct
+                    do_upgrade = method != DEFAULT_ALG
+                    if do_upgrade:
+                        # upgrade pw hash to default algorithm
+                        data['enc_password'] = encodePassword(password)
+                    return True, do_upgrade
+
+                # wrong password
                 return False, False
 
-        if epwd[:6] == '{SSHA}':
-            data = base64.decodestring(epwd[6:])
-            salt = data[20:]
-            hash = hash_new('sha1', password.encode('utf-8'))
-            hash.update(salt)
-            return safe_str_equal(hash.digest(), data[:20]), False
-
-        # No encoded password match, this must be wrong password
+        # unsupported algorithm
         return False, False
 
     def persistent_items(self):