Permalink
Browse files

Add configured password hash test back and fix bug with checking pass…

…words
  • Loading branch information...
mattupstate committed Jan 14, 2014
1 parent 76fc578 commit 35fd08772b05f0120ed883f780e3299aa008c0be
Showing with 18 additions and 15 deletions.
  1. +6 −3 flask_security/utils.py
  2. +12 −12 tests/configured_tests.py
View
@@ -121,7 +121,10 @@ def verify_and_update_password(password, user):
:param password: A plaintext password to verify
:param user: The user to verify against
"""
- verified, new_password = _pwd_context.verify_and_update(encrypt_password(password), user.password)
+
+ if _security.password_hash != 'plaintext':
+ password = get_hmac(password)
+ verified, new_password = _pwd_context.verify_and_update(password, user.password)
@maebert

maebert Feb 5, 2014

Contributor

@mattupstate Can I ask why you made this change? get_hmac and encrypt_password can do very different things, and this leads to the situation where verify_and_update_password may pass but verify_password doesn't. Why the different treatment here?

@mattupstate

mattupstate Feb 5, 2014

Owner

Because the associated test did not pass without making this change.

@maebert

maebert Feb 5, 2014

Contributor

But why here and not in verify_password, too? I just don't understand what underlying problem got fixed here.

@mattupstate

mattupstate Feb 5, 2014

Owner

You've just made me realize that the verify_password function is not used at all within the extension. Nor is it tested. If you're using it, I would suggest using verify_and_update_password instead.

@maebert

maebert Feb 5, 2014

Contributor

Yup, I just realised that myself... we're only using it once, when changing a password (where updating the old password it just before the change doesn't make much sense.) I guess those methods should just be somewhat aligned.

if verified and new_password:
user.password = new_password
_datastore.put(user)
@@ -135,8 +138,8 @@ def encrypt_password(password):
"""
if _security.password_hash == 'plaintext':
return password
- signed = get_hmac(password)
- return _pwd_context.encrypt(signed.decode('ascii'))
+ signed = get_hmac(password).decode('ascii')
+ return _pwd_context.encrypt(signed)
def md5(data):
View
@@ -19,18 +19,18 @@
from tests import SecurityTest
-# TODO: Wait for passlib + bcrypt python3 compatibility to be fixed
-# class ConfiguredPasswordHashSecurityTests(SecurityTest):
-
-# AUTH_CONFIG = {
-# 'SECURITY_PASSWORD_HASH': 'bcrypt',
-# 'SECURITY_PASSWORD_SALT': 'so-salty',
-# 'USER_COUNT': 1
-# }
-
-# def test_authenticate(self):
-# r = self.authenticate(endpoint="/login")
-# self.assertIn(b'Home Page', r.data)
+
+class ConfiguredPasswordHashSecurityTests(SecurityTest):
+
+ AUTH_CONFIG = {
+ 'SECURITY_PASSWORD_HASH': 'bcrypt',
+ 'SECURITY_PASSWORD_SALT': 'so-salty',
+ 'USER_COUNT': 1
+ }
+
+ def test_authenticate(self):
+ r = self.authenticate(endpoint="/login")
+ self.assertIn(b'Home Page', r.data)
class ConfiguredSecurityTests(SecurityTest):

0 comments on commit 35fd087

Please sign in to comment.