Reject rounds=0 for SHA1 hashes (#361)

Port of firebase/firebase-admin-node#677

Author: rsgowman

firebase_admin/_user_import.py

@@ -282,11 +282,6 @@ def _hmac(cls, name, key):
         }
         return UserImportHash(name, data)
 
-    @classmethod
-    def _basic_hash(cls, name, rounds):
-        data = {'rounds': _auth_utils.validate_int(rounds, 'rounds', 0, 120000)}
-        return UserImportHash(name, data)
-
     @classmethod
     def hmac_sha512(cls, key):
         """Creates a new HMAC SHA512 algorithm instance.
@@ -340,48 +335,56 @@ def md5(cls, rounds):
         """Creates a new MD5 algorithm instance.
 
         Args:
-            rounds: Number of rounds. Must be an integer between 0 and 120000.
+            rounds: Number of rounds. Must be an integer between 0 and 8192.
 
         Returns:
             UserImportHash: A new ``UserImportHash``.
         """
-        return cls._basic_hash('MD5', rounds)
+        return UserImportHash(
+            'MD5',
+            {'rounds': _auth_utils.validate_int(rounds, 'rounds', 0, 8192)})
 
     @classmethod
     def sha1(cls, rounds):
         """Creates a new SHA1 algorithm instance.
 
         Args:
-            rounds: Number of rounds. Must be an integer between 0 and 120000.
+            rounds: Number of rounds. Must be an integer between 1 and 8192.
 
         Returns:
             UserImportHash: A new ``UserImportHash``.
         """
-        return cls._basic_hash('SHA1', rounds)
+        return UserImportHash(
+            'SHA1',
+            {'rounds': _auth_utils.validate_int(rounds, 'rounds', 1, 8192)})
 
     @classmethod
     def sha256(cls, rounds):
         """Creates a new SHA256 algorithm instance.
 
         Args:
-            rounds: Number of rounds. Must be an integer between 0 and 120000.
+            rounds: Number of rounds. Must be an integer between 1 and 8192.
 
         Returns:
             UserImportHash: A new ``UserImportHash``.
         """
-        return cls._basic_hash('SHA256', rounds)
+        return UserImportHash(
+            'SHA256',
+            {'rounds': _auth_utils.validate_int(rounds, 'rounds', 1, 8192)})
 
     @classmethod
     def sha512(cls, rounds):
         """Creates a new SHA512 algorithm instance.
 
         Args:
-            rounds: Number of rounds. Must be an integer between 0 and 120000.
+            rounds: Number of rounds. Must be an integer between 1 and 8192.
 
         Returns:
             UserImportHash: A new ``UserImportHash``.
         """
-        return cls._basic_hash('SHA512', rounds)
+        return UserImportHash(
+            'SHA512',
+            {'rounds': _auth_utils.validate_int(rounds, 'rounds', 1, 8192)})
 
     @classmethod
     def pbkdf_sha1(cls, rounds):
@@ -393,7 +396,9 @@ def pbkdf_sha1(cls, rounds):
         Returns:
             UserImportHash: A new ``UserImportHash``.
         """
-        return cls._basic_hash('PBKDF_SHA1', rounds)
+        return UserImportHash(
+            'PBKDF_SHA1',
+            {'rounds': _auth_utils.validate_int(rounds, 'rounds', 0, 120000)})
 
     @classmethod
     def pbkdf2_sha256(cls, rounds):
@@ -405,7 +410,9 @@ def pbkdf2_sha256(cls, rounds):
         Returns:
             UserImportHash: A new ``UserImportHash``.
         """
-        return cls._basic_hash('PBKDF2_SHA256', rounds)
+        return UserImportHash(
+            'PBKDF2_SHA256',
+            {'rounds': _auth_utils.validate_int(rounds, 'rounds', 0, 120000)})
 
     @classmethod
     def scrypt(cls, key, rounds, memory_cost, salt_separator=None):

tests/test_user_mgt.py

@@ -933,31 +933,35 @@ def test_invalid_hmac(self, func, key):
         with pytest.raises(ValueError):
             func(key=key)
 
-    @pytest.mark.parametrize('func,name', [
-        (auth.UserImportHash.sha512, 'SHA512'),
-        (auth.UserImportHash.sha256, 'SHA256'),
-        (auth.UserImportHash.sha1, 'SHA1'),
-        (auth.UserImportHash.md5, 'MD5'),
-        (auth.UserImportHash.pbkdf_sha1, 'PBKDF_SHA1'),
-        (auth.UserImportHash.pbkdf2_sha256, 'PBKDF2_SHA256'),
+    @pytest.mark.parametrize('func,name,rounds', [
+        (auth.UserImportHash.md5, 'MD5', [0, 8192]),
+        (auth.UserImportHash.sha1, 'SHA1', [1, 8192]),
+        (auth.UserImportHash.sha256, 'SHA256', [1, 8192]),
+        (auth.UserImportHash.sha512, 'SHA512', [1, 8192]),
+        (auth.UserImportHash.pbkdf_sha1, 'PBKDF_SHA1', [0, 120000]),
+        (auth.UserImportHash.pbkdf2_sha256, 'PBKDF2_SHA256', [0, 120000]),
     ])
-    def test_basic(self, func, name):
-        basic = func(rounds=10)
-        expected = {
-            'hashAlgorithm': name,
-            'rounds': 10,
-        }
-        assert basic.to_dict() == expected
-
-    @pytest.mark.parametrize('func', [
-        auth.UserImportHash.sha512, auth.UserImportHash.sha256,
-        auth.UserImportHash.sha1, auth.UserImportHash.md5,
-        auth.UserImportHash.pbkdf_sha1, auth.UserImportHash.pbkdf2_sha256,
+    def test_basic(self, func, name, rounds):
+        for rnds in rounds:
+            basic = func(rounds=rnds)
+            expected = {
+                'hashAlgorithm': name,
+                'rounds': rnds,
+            }
+            assert basic.to_dict() == expected
+
+    @pytest.mark.parametrize('func,rounds', [
+        (auth.UserImportHash.md5, INVALID_INTS + [-1, 8193]),
+        (auth.UserImportHash.sha1, INVALID_INTS + [0, 8193]),
+        (auth.UserImportHash.sha256, INVALID_INTS + [0, 8193]),
+        (auth.UserImportHash.sha512, INVALID_INTS + [0, 8193]),
+        (auth.UserImportHash.pbkdf_sha1, INVALID_INTS + [-1, 120001]),
+        (auth.UserImportHash.pbkdf2_sha256, INVALID_INTS + [-1, 120001]),
     ])
-    @pytest.mark.parametrize('rounds', INVALID_INTS + [120001])
     def test_invalid_basic(self, func, rounds):
-        with pytest.raises(ValueError):
-            func(rounds=rounds)
+        for rnds in rounds:
+            with pytest.raises(ValueError):
+                func(rounds=rnds)
 
     def test_scrypt(self):
         scrypt = auth.UserImportHash.scrypt(