Add password reset request handling to the account page
The next step in the password reset process (confirmation) continues to be handled by student.views.password_reset_confirm_wrapper, a custom wrapper around Django's password reset confirmation view.
This commit is contained in:
@@ -5,8 +5,11 @@ address, but does NOT include user profile information (i.e., demographic
|
||||
information and preferences).
|
||||
|
||||
"""
|
||||
from django.conf import settings
|
||||
from django.db import transaction, IntegrityError
|
||||
from django.core.validators import validate_email, validate_slug, ValidationError
|
||||
from django.contrib.auth.forms import PasswordResetForm
|
||||
|
||||
from user_api.models import User, UserProfile, Registration, PendingEmailChange
|
||||
from user_api.helpers import intercept_errors
|
||||
|
||||
@@ -300,6 +303,43 @@ def confirm_email_change(activation_key):
|
||||
return (old_email, new_email)
|
||||
|
||||
|
||||
@intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError])
|
||||
def request_password_change(email, orig_host, is_secure):
|
||||
"""Email a single-use link for performing a password reset.
|
||||
|
||||
Users must confirm the password change before we update their information.
|
||||
|
||||
Args:
|
||||
email (string): An email address
|
||||
orig_host (string): An originating host, extracted from a request with get_host
|
||||
is_secure (Boolean): Whether the request was made with HTTPS
|
||||
|
||||
Returns:
|
||||
None
|
||||
|
||||
Raises:
|
||||
AccountUserNotFound
|
||||
AccountRequestError
|
||||
|
||||
"""
|
||||
# Binding data to a form requires that the data be passed as a dictionary
|
||||
# to the Form class constructor.
|
||||
form = PasswordResetForm({'email': email})
|
||||
|
||||
# Validate that an active user exists with the given email address.
|
||||
if form.is_valid():
|
||||
# Generate a single-use link for performing a password reset
|
||||
# and email it to the user.
|
||||
form.save(
|
||||
from_email=settings.DEFAULT_FROM_EMAIL,
|
||||
domain_override=orig_host,
|
||||
use_https=is_secure
|
||||
)
|
||||
else:
|
||||
# No active user with the provided email address exists.
|
||||
raise AccountUserNotFound
|
||||
|
||||
|
||||
def _validate_username(username):
|
||||
"""Validate the username.
|
||||
|
||||
|
||||
@@ -1,12 +1,17 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
""" Tests for the account API. """
|
||||
|
||||
import unittest
|
||||
import re
|
||||
from unittest import skipUnless
|
||||
|
||||
from nose.tools import raises
|
||||
from mock import patch
|
||||
import ddt
|
||||
from dateutil.parser import parse as parse_datetime
|
||||
from django.conf import settings
|
||||
from django.core import mail
|
||||
from django.test import TestCase
|
||||
from django.conf import settings
|
||||
|
||||
from user_api.api import account as account_api
|
||||
from user_api.models import UserProfile
|
||||
|
||||
@@ -14,43 +19,46 @@ from user_api.models import UserProfile
|
||||
@ddt.ddt
|
||||
class AccountApiTest(TestCase):
|
||||
|
||||
USERNAME = u"frank-underwood"
|
||||
PASSWORD = u"ṕáśśẃőŕd"
|
||||
EMAIL = u"frank+underwood@example.com"
|
||||
USERNAME = u'frank-underwood'
|
||||
PASSWORD = u'ṕáśśẃőŕd'
|
||||
EMAIL = u'frank+underwood@example.com'
|
||||
|
||||
ORIG_HOST = 'example.com'
|
||||
IS_SECURE = False
|
||||
|
||||
INVALID_USERNAMES = [
|
||||
None,
|
||||
u"",
|
||||
u"a",
|
||||
u"a" * (account_api.USERNAME_MAX_LENGTH + 1),
|
||||
u"invalid_symbol_@",
|
||||
u"invalid-unicode_fŕáńḱ",
|
||||
u'',
|
||||
u'a',
|
||||
u'a' * (account_api.USERNAME_MAX_LENGTH + 1),
|
||||
u'invalid_symbol_@',
|
||||
u'invalid-unicode_fŕáńḱ',
|
||||
]
|
||||
|
||||
INVALID_EMAILS = [
|
||||
None,
|
||||
u"",
|
||||
u"a",
|
||||
"no_domain",
|
||||
"no+domain",
|
||||
"@",
|
||||
"@domain.com",
|
||||
"test@no_extension",
|
||||
u"fŕáńḱ@example.com",
|
||||
u"frank@éxáḿṕĺé.ćőḿ",
|
||||
u'',
|
||||
u'a',
|
||||
'no_domain',
|
||||
'no+domain',
|
||||
'@',
|
||||
'@domain.com',
|
||||
'test@no_extension',
|
||||
u'fŕáńḱ@example.com',
|
||||
u'frank@éxáḿṕĺé.ćőḿ',
|
||||
|
||||
# Long email -- subtract the length of the @domain
|
||||
# except for one character (so we exceed the max length limit)
|
||||
u"{user}@example.com".format(
|
||||
u'{user}@example.com'.format(
|
||||
user=(u'e' * (account_api.EMAIL_MAX_LENGTH - 11))
|
||||
)
|
||||
]
|
||||
|
||||
INVALID_PASSWORDS = [
|
||||
None,
|
||||
u"",
|
||||
u"a",
|
||||
u"a" * (account_api.PASSWORD_MAX_LENGTH + 1)
|
||||
u'',
|
||||
u'a',
|
||||
u'a' * (account_api.PASSWORD_MAX_LENGTH + 1)
|
||||
]
|
||||
|
||||
def test_activate_account(self):
|
||||
@@ -72,7 +80,7 @@ class AccountApiTest(TestCase):
|
||||
# Request an email change
|
||||
account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
activation_key = account_api.request_email_change(
|
||||
self.USERNAME, u"new+email@example.com", self.PASSWORD
|
||||
self.USERNAME, u'new+email@example.com', self.PASSWORD
|
||||
)
|
||||
|
||||
# Verify that the email has not yet changed
|
||||
@@ -82,24 +90,23 @@ class AccountApiTest(TestCase):
|
||||
# Confirm the change, using the activation code
|
||||
old_email, new_email = account_api.confirm_email_change(activation_key)
|
||||
self.assertEqual(old_email, self.EMAIL)
|
||||
self.assertEqual(new_email, u"new+email@example.com")
|
||||
self.assertEqual(new_email, u'new+email@example.com')
|
||||
|
||||
# Verify that the email is changed
|
||||
account = account_api.account_info(self.USERNAME)
|
||||
self.assertEqual(account['email'], u"new+email@example.com")
|
||||
self.assertEqual(account['email'], u'new+email@example.com')
|
||||
|
||||
def test_confirm_email_change_repeat(self):
|
||||
account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
activation_key = account_api.request_email_change(
|
||||
self.USERNAME, u"new+email@example.com", self.PASSWORD
|
||||
self.USERNAME, u'new+email@example.com', self.PASSWORD
|
||||
)
|
||||
|
||||
# Confirm the change once
|
||||
account_api.confirm_email_change(activation_key)
|
||||
|
||||
# Confirm the change again
|
||||
# The activation code should be single-use
|
||||
# so this should raise an error.
|
||||
# Confirm the change again. The activation code should be
|
||||
# single-use, so this should raise an error.
|
||||
with self.assertRaises(account_api.AccountNotAuthorized):
|
||||
account_api.confirm_email_change(activation_key)
|
||||
|
||||
@@ -110,11 +117,11 @@ class AccountApiTest(TestCase):
|
||||
|
||||
# Email uniqueness constraints were introduced in a database migration,
|
||||
# which we disable in the unit tests to improve the speed of the test suite.
|
||||
@unittest.skipUnless(settings.SOUTH_TESTS_MIGRATE, "South migrations required")
|
||||
@skipUnless(settings.SOUTH_TESTS_MIGRATE, "South migrations required")
|
||||
def test_create_account_duplicate_email(self):
|
||||
account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
with self.assertRaises(account_api.AccountUserAlreadyExists):
|
||||
account_api.create_account("different_user", self.PASSWORD, self.EMAIL)
|
||||
account_api.create_account('different_user', self.PASSWORD, self.EMAIL)
|
||||
|
||||
def test_username_too_long(self):
|
||||
long_username = 'e' * (account_api.USERNAME_MAX_LENGTH + 1)
|
||||
@@ -122,7 +129,7 @@ class AccountApiTest(TestCase):
|
||||
account_api.create_account(long_username, self.PASSWORD, self.EMAIL)
|
||||
|
||||
def test_account_info_no_user(self):
|
||||
self.assertIs(account_api.account_info("does_not_exist"), None)
|
||||
self.assertIs(account_api.account_info('does_not_exist'), None)
|
||||
|
||||
@raises(account_api.AccountEmailInvalid)
|
||||
@ddt.data(*INVALID_EMAILS)
|
||||
@@ -146,11 +153,11 @@ class AccountApiTest(TestCase):
|
||||
|
||||
@raises(account_api.AccountNotAuthorized)
|
||||
def test_activate_account_invalid_key(self):
|
||||
account_api.activate_account(u"invalid")
|
||||
account_api.activate_account(u'invalid')
|
||||
|
||||
@raises(account_api.AccountUserNotFound)
|
||||
def test_request_email_change_no_user(self):
|
||||
account_api.request_email_change(u"no_such_user", self.EMAIL, self.PASSWORD)
|
||||
account_api.request_email_change(u'no_such_user', self.EMAIL, self.PASSWORD)
|
||||
|
||||
@ddt.data(*INVALID_EMAILS)
|
||||
def test_request_email_change_invalid_email(self, invalid_email):
|
||||
@@ -165,22 +172,22 @@ class AccountApiTest(TestCase):
|
||||
# Create two accounts, both activated
|
||||
activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
account_api.activate_account(activation_key)
|
||||
activation_key = account_api.create_account(u"another_user", u"password", u"another+user@example.com")
|
||||
activation_key = account_api.create_account(u'another_user', u'password', u'another+user@example.com')
|
||||
account_api.activate_account(activation_key)
|
||||
|
||||
# Try to change the first user's email to the same as the second user's
|
||||
with self.assertRaises(account_api.AccountEmailAlreadyExists):
|
||||
account_api.request_email_change(self.USERNAME, u"another+user@example.com", self.PASSWORD)
|
||||
account_api.request_email_change(self.USERNAME, u'another+user@example.com', self.PASSWORD)
|
||||
|
||||
def test_request_email_change_duplicates_unactivated_account(self):
|
||||
# Create two accounts, but the second account is inactive
|
||||
activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
account_api.activate_account(activation_key)
|
||||
account_api.create_account(u"another_user", u"password", u"another+user@example.com")
|
||||
account_api.create_account(u'another_user', u'password', u'another+user@example.com')
|
||||
|
||||
# Try to change the first user's email to the same as the second user's
|
||||
# Since the second user has not yet activated, this should succeed.
|
||||
account_api.request_email_change(self.USERNAME, u"another+user@example.com", self.PASSWORD)
|
||||
account_api.request_email_change(self.USERNAME, u'another+user@example.com', self.PASSWORD)
|
||||
|
||||
def test_request_email_change_same_address(self):
|
||||
# Create and activate the account
|
||||
@@ -196,14 +203,14 @@ class AccountApiTest(TestCase):
|
||||
|
||||
# Use the wrong password
|
||||
with self.assertRaises(account_api.AccountNotAuthorized):
|
||||
account_api.request_email_change(self.USERNAME, u"new+email@example.com", u"wrong password")
|
||||
account_api.request_email_change(self.USERNAME, u'new+email@example.com', u'wrong password')
|
||||
|
||||
def test_confirm_email_change_invalid_activation_key(self):
|
||||
account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
account_api.request_email_change(self.USERNAME, u"new+email@example.com", self.PASSWORD)
|
||||
account_api.request_email_change(self.USERNAME, u'new+email@example.com', self.PASSWORD)
|
||||
|
||||
with self.assertRaises(account_api.AccountNotAuthorized):
|
||||
account_api.confirm_email_change(u"invalid")
|
||||
account_api.confirm_email_change(u'invalid')
|
||||
|
||||
def test_confirm_email_change_no_request_pending(self):
|
||||
account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
@@ -213,11 +220,11 @@ class AccountApiTest(TestCase):
|
||||
|
||||
# Request a change
|
||||
activation_key = account_api.request_email_change(
|
||||
self.USERNAME, u"new+email@example.com", self.PASSWORD
|
||||
self.USERNAME, u'new+email@example.com', self.PASSWORD
|
||||
)
|
||||
|
||||
# Another use takes the email before we confirm the change
|
||||
account_api.create_account(u"other_user", u"password", u"new+email@example.com")
|
||||
account_api.create_account(u'other_user', u'password', u'new+email@example.com')
|
||||
|
||||
# When we try to confirm our change, we get an error because the email is taken
|
||||
with self.assertRaises(account_api.AccountEmailAlreadyExists):
|
||||
@@ -229,7 +236,7 @@ class AccountApiTest(TestCase):
|
||||
def test_confirm_email_no_user_profile(self):
|
||||
account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
activation_key = account_api.request_email_change(
|
||||
self.USERNAME, u"new+email@example.com", self.PASSWORD
|
||||
self.USERNAME, u'new+email@example.com', self.PASSWORD
|
||||
)
|
||||
|
||||
# This should never happen, but just in case...
|
||||
@@ -243,7 +250,7 @@ class AccountApiTest(TestCase):
|
||||
|
||||
# Change the email once
|
||||
activation_key = account_api.request_email_change(
|
||||
self.USERNAME, u"new+email@example.com", self.PASSWORD
|
||||
self.USERNAME, u'new+email@example.com', self.PASSWORD
|
||||
)
|
||||
account_api.confirm_email_change(activation_key)
|
||||
|
||||
@@ -256,7 +263,7 @@ class AccountApiTest(TestCase):
|
||||
|
||||
# Change the email again
|
||||
activation_key = account_api.request_email_change(
|
||||
self.USERNAME, u"another_new+email@example.com", self.PASSWORD
|
||||
self.USERNAME, u'another_new+email@example.com', self.PASSWORD
|
||||
)
|
||||
account_api.confirm_email_change(activation_key)
|
||||
|
||||
@@ -264,9 +271,39 @@ class AccountApiTest(TestCase):
|
||||
meta = UserProfile.objects.get(user__username=self.USERNAME).get_meta()
|
||||
self.assertEqual(len(meta['old_emails']), 2)
|
||||
email, timestamp = meta['old_emails'][1]
|
||||
self.assertEqual(email, "new+email@example.com")
|
||||
self.assertEqual(email, 'new+email@example.com')
|
||||
self._assert_is_datetime(timestamp)
|
||||
|
||||
@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS')
|
||||
def test_request_password_change(self):
|
||||
# Create and activate an account
|
||||
activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
account_api.activate_account(activation_key)
|
||||
|
||||
# Request a password change
|
||||
account_api.request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE)
|
||||
|
||||
# Verify that one email message has been sent
|
||||
self.assertEqual(len(mail.outbox), 1)
|
||||
|
||||
# Verify that the body of the message contains something that looks
|
||||
# like an activation link
|
||||
email_body = mail.outbox[0].body
|
||||
result = re.search('(?P<url>https?://[^\s]+)', email_body)
|
||||
self.assertIsNot(result, None)
|
||||
|
||||
@raises(account_api.AccountUserNotFound)
|
||||
@ddt.data(True, False)
|
||||
def test_request_password_change_invalid_user(self, create_inactive_account):
|
||||
if create_inactive_account:
|
||||
# Create an account, but do not activate it
|
||||
account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
|
||||
account_api.request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE)
|
||||
|
||||
# Verify that no email messages have been sent
|
||||
self.assertEqual(len(mail.outbox), 0)
|
||||
|
||||
def _assert_is_datetime(self, timestamp):
|
||||
if not timestamp:
|
||||
return False
|
||||
|
||||
@@ -2,7 +2,9 @@
|
||||
""" Tests for student account views. """
|
||||
|
||||
import re
|
||||
from unittest import skipUnless
|
||||
from urllib import urlencode
|
||||
|
||||
from mock import patch
|
||||
import ddt
|
||||
from django.test import TestCase
|
||||
@@ -13,6 +15,7 @@ from django.core import mail
|
||||
from util.testing import UrlResetMixin
|
||||
from user_api.api import account as account_api
|
||||
from user_api.api import profile as profile_api
|
||||
from util.bad_request_rate_limiter import BadRequestRateLimiter
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@@ -21,10 +24,13 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
|
||||
USERNAME = u"heisenberg"
|
||||
ALTERNATE_USERNAME = u"walt"
|
||||
PASSWORD = u"ḅḷüëṡḳÿ"
|
||||
OLD_PASSWORD = u"ḅḷüëṡḳÿ"
|
||||
NEW_PASSWORD = u"🄱🄸🄶🄱🄻🅄🄴"
|
||||
OLD_EMAIL = u"walter@graymattertech.com"
|
||||
NEW_EMAIL = u"walt@savewalterwhite.com"
|
||||
|
||||
INVALID_ATTEMPTS = 100
|
||||
|
||||
INVALID_EMAILS = [
|
||||
None,
|
||||
u"",
|
||||
@@ -49,19 +55,19 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
super(StudentAccountViewTest, self).setUp()
|
||||
|
||||
# Create/activate a new account
|
||||
activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.OLD_EMAIL)
|
||||
activation_key = account_api.create_account(self.USERNAME, self.OLD_PASSWORD, self.OLD_EMAIL)
|
||||
account_api.activate_account(activation_key)
|
||||
|
||||
# Login
|
||||
result = self.client.login(username=self.USERNAME, password=self.PASSWORD)
|
||||
result = self.client.login(username=self.USERNAME, password=self.OLD_PASSWORD)
|
||||
self.assertTrue(result)
|
||||
|
||||
def test_index(self):
|
||||
response = self.client.get(reverse('account_index'))
|
||||
self.assertContains(response, "Student Account")
|
||||
|
||||
def test_change_email(self):
|
||||
response = self._change_email(self.NEW_EMAIL, self.PASSWORD)
|
||||
def test_email_change(self):
|
||||
response = self._change_email(self.NEW_EMAIL, self.OLD_PASSWORD)
|
||||
self.assertEquals(response.status_code, 200)
|
||||
|
||||
# Verify that the email associated with the account remains unchanged
|
||||
@@ -73,8 +79,8 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
self._assert_email(
|
||||
mail.outbox[0],
|
||||
[self.NEW_EMAIL],
|
||||
u'Email Change Request',
|
||||
u'There was recently a request to change the email address'
|
||||
u"Email Change Request",
|
||||
u"There was recently a request to change the email address"
|
||||
)
|
||||
|
||||
# Retrieve the activation key from the email
|
||||
@@ -96,48 +102,48 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
self._assert_email(
|
||||
mail.outbox[1],
|
||||
[self.OLD_EMAIL, self.NEW_EMAIL],
|
||||
u'Email Change Successful',
|
||||
u'You successfully changed the email address'
|
||||
u"Email Change Successful",
|
||||
u"You successfully changed the email address"
|
||||
)
|
||||
|
||||
def test_email_change_wrong_password(self):
|
||||
response = self._change_email(self.NEW_EMAIL, "wrong password")
|
||||
self.assertEqual(response.status_code, 401)
|
||||
|
||||
def test_email_change_request_internal_error(self):
|
||||
def test_email_change_request_no_user(self):
|
||||
# Patch account API to raise an internal error when an email change is requested
|
||||
with patch('student_account.views.account_api.request_email_change') as mock_call:
|
||||
mock_call.side_effect = account_api.AccountUserNotFound
|
||||
response = self._change_email(self.NEW_EMAIL, self.PASSWORD)
|
||||
response = self._change_email(self.NEW_EMAIL, self.OLD_PASSWORD)
|
||||
|
||||
self.assertEquals(response.status_code, 500)
|
||||
self.assertEquals(response.status_code, 400)
|
||||
|
||||
def test_email_change_request_email_taken_by_active_account(self):
|
||||
# Create/activate a second user with the new email
|
||||
activation_key = account_api.create_account(self.ALTERNATE_USERNAME, self.PASSWORD, self.NEW_EMAIL)
|
||||
activation_key = account_api.create_account(self.ALTERNATE_USERNAME, self.OLD_PASSWORD, self.NEW_EMAIL)
|
||||
account_api.activate_account(activation_key)
|
||||
|
||||
# Request to change the original user's email to the email now used by the second user
|
||||
response = self._change_email(self.NEW_EMAIL, self.PASSWORD)
|
||||
response = self._change_email(self.NEW_EMAIL, self.OLD_PASSWORD)
|
||||
self.assertEquals(response.status_code, 409)
|
||||
|
||||
def test_email_change_request_email_taken_by_inactive_account(self):
|
||||
# Create a second user with the new email, but don't active them
|
||||
account_api.create_account(self.ALTERNATE_USERNAME, self.PASSWORD, self.NEW_EMAIL)
|
||||
account_api.create_account(self.ALTERNATE_USERNAME, self.OLD_PASSWORD, self.NEW_EMAIL)
|
||||
|
||||
# Request to change the original user's email to the email used by the inactive user
|
||||
response = self._change_email(self.NEW_EMAIL, self.PASSWORD)
|
||||
response = self._change_email(self.NEW_EMAIL, self.OLD_PASSWORD)
|
||||
self.assertEquals(response.status_code, 200)
|
||||
|
||||
@ddt.data(*INVALID_EMAILS)
|
||||
def test_email_change_request_email_invalid(self, invalid_email):
|
||||
# Request to change the user's email to an invalid address
|
||||
response = self._change_email(invalid_email, self.PASSWORD)
|
||||
response = self._change_email(invalid_email, self.OLD_PASSWORD)
|
||||
self.assertEquals(response.status_code, 400)
|
||||
|
||||
def test_email_change_confirmation(self):
|
||||
# Get an email change activation key
|
||||
activation_key = account_api.request_email_change(self.USERNAME, self.NEW_EMAIL, self.PASSWORD)
|
||||
activation_key = account_api.request_email_change(self.USERNAME, self.NEW_EMAIL, self.OLD_PASSWORD)
|
||||
|
||||
# Follow the link sent in the confirmation email
|
||||
response = self.client.get(reverse('email_change_confirm', kwargs={'key': activation_key}))
|
||||
@@ -158,10 +164,10 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
|
||||
def test_email_change_confirmation_email_already_exists(self):
|
||||
# Get an email change activation key
|
||||
email_activation_key = account_api.request_email_change(self.USERNAME, self.NEW_EMAIL, self.PASSWORD)
|
||||
email_activation_key = account_api.request_email_change(self.USERNAME, self.NEW_EMAIL, self.OLD_PASSWORD)
|
||||
|
||||
# Create/activate a second user with the new email
|
||||
account_activation_key = account_api.create_account(self.ALTERNATE_USERNAME, self.PASSWORD, self.NEW_EMAIL)
|
||||
account_activation_key = account_api.create_account(self.ALTERNATE_USERNAME, self.OLD_PASSWORD, self.NEW_EMAIL)
|
||||
account_api.activate_account(account_activation_key)
|
||||
|
||||
# Follow the link sent to the original user
|
||||
@@ -174,7 +180,7 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
|
||||
def test_email_change_confirmation_internal_error(self):
|
||||
# Get an email change activation key
|
||||
activation_key = account_api.request_email_change(self.USERNAME, self.NEW_EMAIL, self.PASSWORD)
|
||||
activation_key = account_api.request_email_change(self.USERNAME, self.NEW_EMAIL, self.OLD_PASSWORD)
|
||||
|
||||
# Patch account API to return an internal error
|
||||
with patch('student_account.views.account_api.confirm_email_change') as mock_call:
|
||||
@@ -183,14 +189,120 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
|
||||
self.assertContains(response, "Something went wrong")
|
||||
|
||||
def test_change_email_request_missing_email_param(self):
|
||||
response = self._change_email(None, self.PASSWORD)
|
||||
def test_email_change_request_missing_email_param(self):
|
||||
response = self._change_email(None, self.OLD_PASSWORD)
|
||||
self.assertEqual(response.status_code, 400)
|
||||
|
||||
def test_change_email_request_missing_password_param(self):
|
||||
def test_email_change_request_missing_password_param(self):
|
||||
response = self._change_email(self.OLD_EMAIL, None)
|
||||
self.assertEqual(response.status_code, 400)
|
||||
|
||||
@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS')
|
||||
def test_password_change(self):
|
||||
# Request a password change while logged in, simulating
|
||||
# use of the password reset link from the account page
|
||||
response = self._change_password()
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
# Check that an email was sent
|
||||
self.assertEqual(len(mail.outbox), 1)
|
||||
|
||||
# Retrieve the activation link from the email body
|
||||
email_body = mail.outbox[0].body
|
||||
result = re.search('(?P<url>https?://[^\s]+)', email_body)
|
||||
self.assertIsNot(result, None)
|
||||
activation_link = result.group('url')
|
||||
|
||||
# Visit the activation link
|
||||
response = self.client.get(activation_link)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
# Submit a new password and follow the redirect to the success page
|
||||
response = self.client.post(
|
||||
activation_link,
|
||||
# These keys are from the form on the current password reset confirmation page.
|
||||
{'new_password1': self.NEW_PASSWORD, 'new_password2': self.NEW_PASSWORD},
|
||||
follow=True
|
||||
)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertContains(response, "Your password has been set.")
|
||||
|
||||
# Log the user out to clear session data
|
||||
self.client.logout()
|
||||
|
||||
# Verify that the new password can be used to log in
|
||||
result = self.client.login(username=self.USERNAME, password=self.NEW_PASSWORD)
|
||||
self.assertTrue(result)
|
||||
|
||||
# Try reusing the activation link to change the password again
|
||||
response = self.client.post(
|
||||
activation_link,
|
||||
{'new_password1': self.OLD_PASSWORD, 'new_password2': self.OLD_PASSWORD},
|
||||
follow=True
|
||||
)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertContains(response, "The password reset link was invalid, possibly because the link has already been used.")
|
||||
|
||||
self.client.logout()
|
||||
|
||||
# Verify that the old password cannot be used to log in
|
||||
result = self.client.login(username=self.USERNAME, password=self.OLD_PASSWORD)
|
||||
self.assertFalse(result)
|
||||
|
||||
# Verify that the new password continues to be valid
|
||||
result = self.client.login(username=self.USERNAME, password=self.NEW_PASSWORD)
|
||||
self.assertTrue(result)
|
||||
|
||||
|
||||
@ddt.data(True, False)
|
||||
def test_password_change_logged_out(self, send_email):
|
||||
# Log the user out
|
||||
self.client.logout()
|
||||
|
||||
# Request a password change while logged out, simulating
|
||||
# use of the password reset link from the login page
|
||||
if send_email:
|
||||
response = self._change_password(email=self.OLD_EMAIL)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
else:
|
||||
# Don't send an email in the POST data, simulating
|
||||
# its (potentially accidental) omission in the POST
|
||||
# data sent from the login page
|
||||
response = self._change_password()
|
||||
self.assertEqual(response.status_code, 400)
|
||||
|
||||
def test_password_change_inactive_user(self):
|
||||
# Log out the user created during test setup
|
||||
self.client.logout()
|
||||
|
||||
# Create a second user, but do not activate it
|
||||
account_api.create_account(self.ALTERNATE_USERNAME, self.OLD_PASSWORD, self.NEW_EMAIL)
|
||||
|
||||
# Send the view the email address tied to the inactive user
|
||||
response = self._change_password(email=self.NEW_EMAIL)
|
||||
self.assertEqual(response.status_code, 400)
|
||||
|
||||
def test_password_change_no_user(self):
|
||||
# Log out the user created during test setup
|
||||
self.client.logout()
|
||||
|
||||
# Send the view an email address not tied to any user
|
||||
response = self._change_password(email=self.NEW_EMAIL)
|
||||
self.assertEqual(response.status_code, 400)
|
||||
|
||||
def test_password_change_rate_limited(self):
|
||||
# Log out the user created during test setup, to prevent the view from
|
||||
# selecting the logged-in user's email address over the email provided
|
||||
# in the POST data
|
||||
self.client.logout()
|
||||
|
||||
# Make many consecutive bad requests in an attempt to trigger the rate limiter
|
||||
for attempt in xrange(self.INVALID_ATTEMPTS):
|
||||
self._change_password(email=self.NEW_EMAIL)
|
||||
|
||||
response = self._change_password(email=self.NEW_EMAIL)
|
||||
self.assertEqual(response.status_code, 403)
|
||||
|
||||
@ddt.data(
|
||||
('get', 'account_index', []),
|
||||
('post', 'email_change_request', []),
|
||||
@@ -210,7 +322,8 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
@ddt.data(
|
||||
('get', 'account_index', []),
|
||||
('post', 'email_change_request', []),
|
||||
('get', 'email_change_confirm', [123])
|
||||
('get', 'email_change_confirm', [123]),
|
||||
('post', 'password_change_request', []),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_require_http_method(self, correct_method, url_name, args):
|
||||
@@ -238,3 +351,12 @@ class StudentAccountViewTest(UrlResetMixin, TestCase):
|
||||
data['password'] = password.encode('utf-8')
|
||||
|
||||
return self.client.post(path=reverse('email_change_request'), data=data)
|
||||
|
||||
def _change_password(self, email=None):
|
||||
"""Request to change the user's password. """
|
||||
data = {}
|
||||
|
||||
if email:
|
||||
data['email'] = email
|
||||
|
||||
return self.client.post(path=reverse('password_change_request'), data=data)
|
||||
|
||||
@@ -5,4 +5,5 @@ urlpatterns = patterns(
|
||||
url(r'^$', 'index', name='account_index'),
|
||||
url(r'^email$', 'email_change_request_handler', name='email_change_request'),
|
||||
url(r'^email/confirmation/(?P<key>[^/]*)$', 'email_change_confirmation_handler', name='email_change_confirm'),
|
||||
url(r'^password$', 'password_change_request_handler', name='password_change_request'),
|
||||
)
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
""" Views for a student's account information. """
|
||||
|
||||
import logging
|
||||
|
||||
from django.conf import settings
|
||||
from django.http import (
|
||||
QueryDict, HttpResponse,
|
||||
HttpResponseBadRequest, HttpResponseServerError
|
||||
HttpResponse, HttpResponseBadRequest,
|
||||
HttpResponseForbidden
|
||||
)
|
||||
from django.core.mail import send_mail
|
||||
from django_future.csrf import ensure_csrf_cookie
|
||||
@@ -14,6 +16,10 @@ from microsite_configuration import microsite
|
||||
|
||||
from user_api.api import account as account_api
|
||||
from user_api.api import profile as profile_api
|
||||
from util.bad_request_rate_limiter import BadRequestRateLimiter
|
||||
|
||||
|
||||
AUDIT_LOG = logging.getLogger("audit")
|
||||
|
||||
|
||||
@login_required
|
||||
@@ -47,18 +53,20 @@ def index(request):
|
||||
def email_change_request_handler(request):
|
||||
"""Handle a request to change the user's email address.
|
||||
|
||||
Sends an email to the newly specified address containing a link
|
||||
to a confirmation page.
|
||||
|
||||
Args:
|
||||
request (HttpRequest)
|
||||
|
||||
Returns:
|
||||
HttpResponse: 200 if the confirmation email was sent successfully
|
||||
HttpResponse: 302 if not logged in (redirect to login page)
|
||||
HttpResponse: 400 if the format of the new email is incorrect
|
||||
HttpResponse: 400 if the format of the new email is incorrect, or if
|
||||
an email change is requested for a user which does not exist
|
||||
HttpResponse: 401 if the provided password (in the form) is incorrect
|
||||
HttpResponse: 405 if using an unsupported HTTP method
|
||||
HttpResponse: 409 if the provided email is already in use
|
||||
HttpResponse: 500 if the user to which the email change will be applied
|
||||
does not exist
|
||||
|
||||
Example usage:
|
||||
|
||||
@@ -78,12 +86,10 @@ def email_change_request_handler(request):
|
||||
|
||||
try:
|
||||
key = account_api.request_email_change(username, new_email, password)
|
||||
except account_api.AccountUserNotFound:
|
||||
return HttpResponseServerError()
|
||||
except (account_api.AccountEmailInvalid, account_api.AccountUserNotFound):
|
||||
return HttpResponseBadRequest()
|
||||
except account_api.AccountEmailAlreadyExists:
|
||||
return HttpResponse(status=409)
|
||||
except account_api.AccountEmailInvalid:
|
||||
return HttpResponseBadRequest()
|
||||
except account_api.AccountNotAuthorized:
|
||||
return HttpResponse(status=401)
|
||||
|
||||
@@ -105,7 +111,6 @@ def email_change_request_handler(request):
|
||||
# Send a confirmation email to the new address containing the activation key
|
||||
send_mail(subject, message, from_address, [new_email])
|
||||
|
||||
# Send a 200 response code to the client to indicate that the email was sent successfully.
|
||||
return HttpResponse(status=200)
|
||||
|
||||
|
||||
@@ -122,15 +127,15 @@ def email_change_confirmation_handler(request, key):
|
||||
|
||||
Returns:
|
||||
HttpResponse: 200 if the email change is successful, the activation key
|
||||
is invalid, the new email is already in use, or the
|
||||
user to which the email change will be applied does
|
||||
not exist
|
||||
is invalid, the new email is already in use, or the
|
||||
user to which the email change will be applied does
|
||||
not exist
|
||||
HttpResponse: 302 if not logged in (redirect to login page)
|
||||
HttpResponse: 405 if using an unsupported HTTP method
|
||||
|
||||
Example usage:
|
||||
|
||||
GET /account/email_change_confirm/{key}
|
||||
GET /account/email/confirmation/{key}
|
||||
|
||||
"""
|
||||
try:
|
||||
@@ -179,3 +184,53 @@ def email_change_confirmation_handler(request, key):
|
||||
'disable_courseware_js': True,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@require_http_methods(['POST'])
|
||||
def password_change_request_handler(request):
|
||||
"""Handle password change requests originating from the account page.
|
||||
|
||||
Uses the Account API to email the user a link to the password reset page.
|
||||
|
||||
Note:
|
||||
The next step in the password reset process (confirmation) is currently handled
|
||||
by student.views.password_reset_confirm_wrapper, a custom wrapper around Django's
|
||||
password reset confirmation view.
|
||||
|
||||
Args:
|
||||
request (HttpRequest)
|
||||
|
||||
Returns:
|
||||
HttpResponse: 200 if the email was sent successfully
|
||||
HttpResponse: 400 if there is no 'email' POST parameter, or if no user with
|
||||
the provided email exists
|
||||
HttpResponse: 403 if the client has been rate limited
|
||||
HttpResponse: 405 if using an unsupported HTTP method
|
||||
|
||||
Example usage:
|
||||
|
||||
POST /account/password
|
||||
|
||||
"""
|
||||
limiter = BadRequestRateLimiter()
|
||||
if limiter.is_rate_limit_exceeded(request):
|
||||
AUDIT_LOG.warning("Password reset rate limit exceeded")
|
||||
return HttpResponseForbidden()
|
||||
|
||||
user = request.user
|
||||
# Prefer logged-in user's email
|
||||
email = user.email if user.is_authenticated() else request.POST.get('email')
|
||||
|
||||
if email:
|
||||
try:
|
||||
account_api.request_password_change(email, request.get_host(), request.is_secure())
|
||||
except account_api.AccountUserNotFound:
|
||||
AUDIT_LOG.info("Invalid password reset attempt")
|
||||
# Increment the rate limit counter
|
||||
limiter.tick_bad_request_counter(request)
|
||||
|
||||
return HttpResponseBadRequest("No active user with the provided email address exists.")
|
||||
|
||||
return HttpResponse(status=200)
|
||||
else:
|
||||
return HttpResponseBadRequest("No email address provided.")
|
||||
|
||||
@@ -97,6 +97,11 @@ define(['js/student_account/account'],
|
||||
view.submit(fakeEvent);
|
||||
};
|
||||
|
||||
var requestPasswordChange = function() {
|
||||
var fakeEvent = {preventDefault: function() {}};
|
||||
view.click(fakeEvent);
|
||||
};
|
||||
|
||||
var assertAjax = function(url, method, data) {
|
||||
expect($.ajax).toHaveBeenCalled();
|
||||
var ajaxArgs = $.ajax.mostRecentCall.args[0];
|
||||
@@ -106,31 +111,13 @@ define(['js/student_account/account'],
|
||||
expect(ajaxArgs.headers.hasOwnProperty("X-CSRFToken")).toBe(true);
|
||||
};
|
||||
|
||||
var assertEmailStatus = function(success, expectedStatus) {
|
||||
var assertStatus = function(selection, success, errorClass, expectedStatus) {
|
||||
if (!success) {
|
||||
expect(view.$emailStatus).toHaveClass("validation-error");
|
||||
expect(selection).toHaveClass(errorClass);
|
||||
} else {
|
||||
expect(view.$emailStatus).not.toHaveClass("validation-error");
|
||||
expect(selection).not.toHaveClass(errorClass);
|
||||
}
|
||||
expect(view.$emailStatus.text()).toEqual(expectedStatus);
|
||||
};
|
||||
|
||||
var assertPasswordStatus = function(success, expectedStatus) {
|
||||
if (!success) {
|
||||
expect(view.$passwordStatus).toHaveClass("validation-error");
|
||||
} else {
|
||||
expect(view.$passwordStatus).not.toHaveClass("validation-error");
|
||||
}
|
||||
expect(view.$passwordStatus.text()).toEqual(expectedStatus);
|
||||
};
|
||||
|
||||
var assertRequestStatus = function(success, expectedStatus) {
|
||||
if (!success) {
|
||||
expect(view.$requestStatus).toHaveClass("error");
|
||||
} else {
|
||||
expect(view.$requestStatus).not.toHaveClass("error");
|
||||
}
|
||||
expect(view.$requestStatus.text()).toEqual(expectedStatus);
|
||||
expect(selection.text()).toEqual(expectedStatus);
|
||||
};
|
||||
|
||||
beforeEach(function() {
|
||||
@@ -139,7 +126,7 @@ define(['js/student_account/account'],
|
||||
|
||||
view = new edx.student.account.AccountView().render();
|
||||
|
||||
// Stub Ajax cals to return success/failure
|
||||
// Stub Ajax calls to return success/failure
|
||||
spyOn($, "ajax").andCallFake(function() {
|
||||
return $.Deferred(function(defer) {
|
||||
if (ajaxSuccess) {
|
||||
@@ -157,39 +144,57 @@ define(['js/student_account/account'],
|
||||
email: "bob@example.com",
|
||||
password: "password"
|
||||
});
|
||||
assertRequestStatus(true, "Please check your email to confirm the change");
|
||||
assertStatus(view.$requestStatus, true, "error", "Please check your email to confirm the change");
|
||||
});
|
||||
|
||||
it("displays email validation errors", function() {
|
||||
// Invalid email should display an error
|
||||
requestEmailChange("invalid", "password");
|
||||
assertEmailStatus(false, "Please enter a valid email address");
|
||||
assertStatus(view.$emailStatus, false, "validation-error", "Please enter a valid email address");
|
||||
|
||||
// Once the error is fixed, the status should return to normal
|
||||
requestEmailChange("bob@example.com", "password");
|
||||
assertEmailStatus(true, "");
|
||||
assertStatus(view.$emailStatus, true, "validation-error", "");
|
||||
});
|
||||
|
||||
it("displays an invalid password error", function() {
|
||||
// Password cannot be empty
|
||||
requestEmailChange("bob@example.com", "");
|
||||
assertPasswordStatus(false, "Please enter a valid password");
|
||||
assertStatus(view.$passwordStatus, false, "validation-error", "Please enter a valid password");
|
||||
|
||||
// Once the error is fixed, the status should return to normal
|
||||
requestEmailChange("bob@example.com", "password");
|
||||
assertPasswordStatus(true, "");
|
||||
assertStatus(view.$passwordStatus, true, "validation-error", "");
|
||||
});
|
||||
|
||||
it("displays server errors", function() {
|
||||
// Simulate an error from the server
|
||||
ajaxSuccess = false;
|
||||
requestEmailChange("bob@example.com", "password");
|
||||
assertRequestStatus(false, "The data could not be saved.");
|
||||
assertStatus(view.$requestStatus, false, "error", "The data could not be saved.");
|
||||
|
||||
// On retry, it should succeed
|
||||
ajaxSuccess = true;
|
||||
requestEmailChange("bob@example.com", "password");
|
||||
assertRequestStatus(true, "Please check your email to confirm the change");
|
||||
assertStatus(view.$requestStatus, true, "error", "Please check your email to confirm the change");
|
||||
});
|
||||
|
||||
it("requests a password reset", function() {
|
||||
requestPasswordChange();
|
||||
assertAjax("password", "POST", {});
|
||||
assertStatus(view.$passwordResetStatus, true, "error", "Password reset email sent. Follow the link in the email to change your password.");
|
||||
});
|
||||
|
||||
it("displays an error message if a password reset email could not be sent", function() {
|
||||
// Simulate an error from the server
|
||||
ajaxSuccess = false;
|
||||
requestPasswordChange();
|
||||
assertStatus(view.$passwordResetStatus, false, "error", "We weren't able to send you a password reset email.");
|
||||
|
||||
// Retry, this time simulating success
|
||||
ajaxSuccess = true;
|
||||
requestPasswordChange();
|
||||
assertStatus(view.$passwordResetStatus, true, "error", "Password reset email sent. Follow the link in the email to change your password.");
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -71,11 +71,12 @@ var edx = edx || {};
|
||||
|
||||
events: {
|
||||
'submit': 'submit',
|
||||
'change': 'change'
|
||||
'change': 'change',
|
||||
'click #password-reset': 'click'
|
||||
},
|
||||
|
||||
initialize: function() {
|
||||
_.bindAll(this, 'render', 'submit', 'change', 'clearStatus', 'invalid', 'error', 'sync');
|
||||
_.bindAll(this, 'render', 'submit', 'change', 'click', 'clearStatus', 'invalid', 'error', 'sync');
|
||||
this.model = new edx.student.account.AccountModel();
|
||||
this.model.on('invalid', this.invalid);
|
||||
this.model.on('error', this.error);
|
||||
@@ -89,6 +90,9 @@ var edx = edx || {};
|
||||
this.$emailStatus = $('#new-email-status', this.$el);
|
||||
this.$passwordStatus = $('#password-status', this.$el);
|
||||
this.$requestStatus = $('#request-email-status', this.$el);
|
||||
this.$passwordReset = $('#password-reset', this.$el);
|
||||
this.$passwordResetStatus = $('#password-reset-status', this.$el);
|
||||
|
||||
return this;
|
||||
},
|
||||
|
||||
@@ -105,6 +109,31 @@ var edx = edx || {};
|
||||
});
|
||||
},
|
||||
|
||||
click: function(event) {
|
||||
event.preventDefault();
|
||||
this.clearStatus();
|
||||
|
||||
self = this;
|
||||
$.ajax({
|
||||
url: 'password',
|
||||
type: 'POST',
|
||||
data: {},
|
||||
headers: {
|
||||
'X-CSRFToken': $.cookie('csrftoken')
|
||||
}
|
||||
})
|
||||
.done(function() {
|
||||
self.$passwordResetStatus
|
||||
.addClass('success')
|
||||
.text(gettext("Password reset email sent. Follow the link in the email to change your password."));
|
||||
})
|
||||
.fail(function() {
|
||||
self.$passwordResetStatus
|
||||
.addClass('error')
|
||||
.text(gettext("We weren't able to send you a password reset email."));
|
||||
});
|
||||
},
|
||||
|
||||
invalid: function(model) {
|
||||
var errors = model.validationError;
|
||||
|
||||
@@ -145,6 +174,10 @@ var edx = edx || {};
|
||||
this.$requestStatus
|
||||
.removeClass('error')
|
||||
.text("");
|
||||
|
||||
this.$passwordResetStatus
|
||||
.removeClass('error')
|
||||
.text("");
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -1,14 +1,19 @@
|
||||
<form id="email-change-form" method="post">
|
||||
<label for="new-email"><%- gettext('New Address') %></label>
|
||||
<label for="new-email"><%- gettext("New Address") %></label>
|
||||
<input id="new-email" type="text" name="new-email" value="" placeholder="xsy@edx.org" data-validate="required email"/>
|
||||
<div id="new-email-status" />
|
||||
|
||||
<label for="password"><%- gettext('Password') %></label>
|
||||
<label for="password"><%- gettext("Password") %></label>
|
||||
<input id="password" type="password" name="password" value="" data-validate="required"/>
|
||||
<div id="password-status" />
|
||||
|
||||
<div class="submit-button">
|
||||
<input type="submit" id="email-change-submit" value="<%- gettext('Change My Email Address') %>">
|
||||
<input type="submit" id="email-change-submit" value="<%- gettext("Change My Email Address") %>">
|
||||
</div>
|
||||
<div id="request-email-status" />
|
||||
|
||||
<div id="password-reset">
|
||||
<a href="#"><%- gettext("Reset Password") %></a>
|
||||
</div>
|
||||
<div id="password-reset-status" />
|
||||
</form>
|
||||
|
||||
@@ -23,4 +23,4 @@
|
||||
|
||||
<p>This is a placeholder for the student's account page.</p>
|
||||
|
||||
<div id="account-container" />
|
||||
<div id="account-container"></div>
|
||||
|
||||
@@ -19,7 +19,7 @@
|
||||
% endfor
|
||||
</%block>
|
||||
|
||||
<h1>${_("Student Profile")}</h1>
|
||||
<h1>Student Profile</h1>
|
||||
|
||||
<p>This is a placeholder for the student's profile page.</p>
|
||||
|
||||
|
||||
Reference in New Issue
Block a user