Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apps/users/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ def clean_email(self):
further information. If you do not receive an email then
please confirm you have entered the same email address used
during account registration."""))
user = self.users_cache[0]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will (softly) clash with PR for https://bugzilla.mozilla.org/show_bug.cgi?id=700599
We just need to keep this in mind when we merge this or the other PR.

if not user.has_usable_password():
raise forms.ValidationError(
_("We can't reset this account's password, please contact the "
"support."))
return email

def save(self, **kw):
Expand Down
8 changes: 8 additions & 0 deletions apps/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,14 @@ def save(self, force_insert=False, force_update=False, using=None, **kwargs):
self.resetcode_expires = datetime.now()
super(UserProfile, self).save(force_insert, force_update, using, **kwargs)

def has_usable_password(self):
"""Override AbstractBaseUser.has_usable_password."""
# We also override the check_password method, and don't rely on
# settings.PASSWORD_HASHERS, and don't use "set_unusable_password", so
# we want to bypass most of AbstractBaseUser.has_usable_password
# checks.
return bool(self.password) # Not None and not empty.

def check_password(self, raw_password):
if '$' not in self.password:
valid = (get_hexdigest('md5', '', raw_password) == self.password)
Expand Down
20 changes: 18 additions & 2 deletions apps/users/tests/test_forms.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import hashlib
from datetime import datetime

from django.contrib.auth.tokens import default_token_generator
Expand Down Expand Up @@ -90,7 +91,7 @@ class TestPasswordResetForm(UserFormBase):

def test_request_fail(self):
r = self.client.post('/en-US/firefox/users/pwreset',
{'email': 'someemail@somedomain.com'})
{'email': 'someemail@somedomain.com'})

eq_(len(mail.outbox), 0)
self.assertFormError(r, 'form', 'email',
Expand All @@ -101,7 +102,22 @@ def test_request_fail(self):

def test_request_success(self):
self.client.post('/en-US/firefox/users/pwreset',
{'email': self.user.email})
{'email': self.user.email})

eq_(len(mail.outbox), 1)
assert mail.outbox[0].subject.find('Password reset') == 0
assert mail.outbox[0].body.find('pwreset/%s' % self.uidb64) > 0

def test_request_success_getpersona_password(self):
"""Email is sent even if the user has no password and the profile has
an "unusable" password according to django's AbstractBaseUser."""
bytes_ = '\xb1\x98og\x88\x87\x08q'
md5 = hashlib.md5('password').hexdigest()
hsh = hashlib.sha512(bytes_ + md5).hexdigest()
self.user.password = 'sha512+MD5$%s$%s' % (bytes, hsh)
self.user.save()
self.client.post('/en-US/firefox/users/pwreset',
{'email': self.user.email})

eq_(len(mail.outbox), 1)
assert mail.outbox[0].subject.find('Password reset') == 0
Expand Down
17 changes: 17 additions & 0 deletions apps/users/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,13 @@ class TestPasswords(amo.tests.TestCase):
def test_invalid_old_password(self):
u = UserProfile(password=self.utf)
assert u.check_password(self.utf) is False
assert u.has_usable_password() is True
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need "is True" given that has_usable_pwd return a boolean?


def test_invalid_new_password(self):
u = UserProfile()
u.set_password(self.utf)
assert u.check_password('wrong') is False
assert u.has_usable_password() is True

def test_valid_old_password(self):
hsh = hashlib.md5(encoding.smart_str(self.utf)).hexdigest()
Expand All @@ -235,55 +237,64 @@ def test_valid_old_password(self):
algo, salt, hsh = u.password.split('$')
eq_(algo, 'sha512')
eq_(hsh, get_hexdigest(algo, salt, self.utf))
assert u.has_usable_password() is True

def test_valid_new_password(self):
u = UserProfile()
u.set_password(self.utf)
assert u.check_password(self.utf) is True
assert u.has_usable_password() is True

def test_persona_sha512_md5(self):
md5 = hashlib.md5('password').hexdigest()
hsh = hashlib.sha512(self.bytes_ + md5).hexdigest()
u = UserProfile(password='sha512+MD5$%s$%s' %
(self.bytes_, hsh))
assert u.check_password('password') is True
assert u.has_usable_password() is True

def test_persona_sha512_base64(self):
hsh = hashlib.sha512(self.bytes_ + 'password').hexdigest()
u = UserProfile(password='sha512+base64$%s$%s' %
(encodestring(self.bytes_), hsh))
assert u.check_password('password') is True
assert u.has_usable_password() is True

def test_persona_sha512_base64_maybe_utf8(self):
hsh = hashlib.sha512(self.bytes_ + self.utf.encode('utf8')).hexdigest()
u = UserProfile(password='sha512+base64$%s$%s' %
(encodestring(self.bytes_), hsh))
assert u.check_password(self.utf) is True
assert u.has_usable_password() is True

def test_persona_sha512_base64_maybe_latin1(self):
passwd = u'fo\xf3'
hsh = hashlib.sha512(self.bytes_ + passwd.encode('latin1')).hexdigest()
u = UserProfile(password='sha512+base64$%s$%s' %
(encodestring(self.bytes_), hsh))
assert u.check_password(passwd) is True
assert u.has_usable_password() is True

def test_persona_sha512_base64_maybe_not_latin1(self):
passwd = u'fo\xf3'
hsh = hashlib.sha512(self.bytes_ + passwd.encode('latin1')).hexdigest()
u = UserProfile(password='sha512+base64$%s$%s' %
(encodestring(self.bytes_), hsh))
assert u.check_password(self.utf) is False
assert u.has_usable_password() is True

def test_persona_sha512_md5_base64(self):
md5 = hashlib.md5('password').hexdigest()
hsh = hashlib.sha512(self.bytes_ + md5).hexdigest()
u = UserProfile(password='sha512+MD5+base64$%s$%s' %
(encodestring(self.bytes_), hsh))
assert u.check_password('password') is True
assert u.has_usable_password() is True

def test_browserid_password(self):
u = UserProfile(password=self.utf, source=amo.LOGIN_SOURCE_UNKNOWN)
assert not u.check_password('foo')
assert u.has_usable_password() is True

@patch('access.acl.action_allowed_user')
def test_needs_tougher_password(self, action_allowed_user):
Expand Down Expand Up @@ -312,6 +323,12 @@ def test_sha512(self):
self.assertTrue(check_password('', blank_encoded))
self.assertFalse(check_password(' ', blank_encoded))

def test_empty_password(self):
profile = UserProfile(password=None)
assert profile.has_usable_password() is False
profile = UserProfile(password='')
assert profile.has_usable_password() is False


class TestBlacklistedUsername(amo.tests.TestCase):
fixtures = ['users/test_backends']
Expand Down