From 0ff1c8fc0c59bc35725ab44fb29ea1b1bcdb0aff Mon Sep 17 00:00:00 2001 From: Mathieu Agopian Date: Wed, 30 Apr 2014 12:43:16 +0200 Subject: [PATCH] sent password reset email even for getpersonas migrated accounts (bug 998477) --- apps/users/forms.py | 5 +++++ apps/users/models.py | 8 ++++++++ apps/users/tests/test_forms.py | 20 ++++++++++++++++++-- apps/users/tests/test_models.py | 17 +++++++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/apps/users/forms.py b/apps/users/forms.py index 88457f470025..a19ca2311f94 100644 --- a/apps/users/forms.py +++ b/apps/users/forms.py @@ -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] + 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): diff --git a/apps/users/models.py b/apps/users/models.py index 63fe008cc0f4..885739fb28db 100644 --- a/apps/users/models.py +++ b/apps/users/models.py @@ -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) diff --git a/apps/users/tests/test_forms.py b/apps/users/tests/test_forms.py index 1c150c96582c..e1335ca147bd 100644 --- a/apps/users/tests/test_forms.py +++ b/apps/users/tests/test_forms.py @@ -1,3 +1,4 @@ +import hashlib from datetime import datetime from django.contrib.auth.tokens import default_token_generator @@ -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', @@ -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 diff --git a/apps/users/tests/test_models.py b/apps/users/tests/test_models.py index d9695cccb3a1..bfc61d0796d0 100644 --- a/apps/users/tests/test_models.py +++ b/apps/users/tests/test_models.py @@ -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 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() @@ -235,11 +237,13 @@ 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() @@ -247,18 +251,21 @@ def test_persona_sha512_md5(self): 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' @@ -266,6 +273,7 @@ def test_persona_sha512_base64_maybe_latin1(self): 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' @@ -273,6 +281,7 @@ def test_persona_sha512_base64_maybe_not_latin1(self): 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() @@ -280,10 +289,12 @@ def test_persona_sha512_md5_base64(self): 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): @@ -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']