diff --git a/common/djangoapps/user_api/api/account.py b/common/djangoapps/user_api/api/account.py
index 4281326860678aafc93137faf1f96e88b7efbc5e..277e1bcb38cfdc1a97d718f565d7c1ec7e8c4300 100644
--- a/common/djangoapps/user_api/api/account.py
+++ b/common/djangoapps/user_api/api/account.py
@@ -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.
diff --git a/common/djangoapps/user_api/tests/test_account_api.py b/common/djangoapps/user_api/tests/test_account_api.py
index 1bcc1bbce8c3373d69f4a54ec6fe23e1ff920b37..2acfbd09cfe1e5d765d398dcd59a3e40fa876f69 100644
--- a/common/djangoapps/user_api/tests/test_account_api.py
+++ b/common/djangoapps/user_api/tests/test_account_api.py
@@ -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
diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py
index 0b2447c3ea42909d6ef2e16475e2eac6db75667a..f3dc650b056f4200708544015ac77db24f3b7ea4 100644
--- a/lms/djangoapps/student_account/test/test_views.py
+++ b/lms/djangoapps/student_account/test/test_views.py
@@ -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)
diff --git a/lms/djangoapps/student_account/urls.py b/lms/djangoapps/student_account/urls.py
index bb5a0d5690d8c987bd17eee7baf8e8c4d73eb0ee..7ffeae55207aee65c30b70876abee701cf142e6a 100644
--- a/lms/djangoapps/student_account/urls.py
+++ b/lms/djangoapps/student_account/urls.py
@@ -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'),
)
diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py
index b20e19d0519c3b9a8e1856d51a03cf2cfd8de4a1..bb6d60cb366007a32c170a2098100683217cda1b 100644
--- a/lms/djangoapps/student_account/views.py
+++ b/lms/djangoapps/student_account/views.py
@@ -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.")
diff --git a/lms/static/js/spec/student_account/account.js b/lms/static/js/spec/student_account/account.js
index 596107f4154a5b638484206b36886fa44ab75546..3262192baf4f04a379b03a07998e6adeeb05d2df 100644
--- a/lms/static/js/spec/student_account/account.js
+++ b/lms/static/js/spec/student_account/account.js
@@ -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.");
});
});
}
diff --git a/lms/static/js/student_account/account.js b/lms/static/js/student_account/account.js
index dda4fba60f9d8ac632197ba8a1d0b581ce221240..21906d9706b771e3c3e511e90a5068bec7fe3a52 100644
--- a/lms/static/js/student_account/account.js
+++ b/lms/static/js/student_account/account.js
@@ -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("");
},
});
diff --git a/lms/templates/student_account/account.underscore b/lms/templates/student_account/account.underscore
index 9a6b91628f9b6944833b6602b15130ec74ad7965..7ba538434f957f6b3fd72db42f3451ff5afa0767 100644
--- a/lms/templates/student_account/account.underscore
+++ b/lms/templates/student_account/account.underscore
@@ -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>
diff --git a/lms/templates/student_account/index.html b/lms/templates/student_account/index.html
index 21ac4f4dda840555cd38dbdb85e7e6abd96624d7..0b9ed36aa577ae75859fdade8ed11e801ca105d1 100644
--- a/lms/templates/student_account/index.html
+++ b/lms/templates/student_account/index.html
@@ -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>
diff --git a/lms/templates/student_profile/index.html b/lms/templates/student_profile/index.html
index aa46c69fcd354799f346c5bbaefdab030c0ba314..5566a46a1d8cd4c6c29cb64268815749763808d3 100644
--- a/lms/templates/student_profile/index.html
+++ b/lms/templates/student_profile/index.html
@@ -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>