Commit 938dcce0 by cahrens Committed by Andy Armstrong

Event change_initiated for password and email change requests.

parent 9ac08ac5
...@@ -5,7 +5,8 @@ import unittest ...@@ -5,7 +5,8 @@ import unittest
from student.tests.factories import UserFactory, RegistrationFactory, PendingEmailChangeFactory from student.tests.factories import UserFactory, RegistrationFactory, PendingEmailChangeFactory
from student.views import ( from student.views import (
reactivation_email_for_user, change_email_request, do_email_change_request, confirm_email_change reactivation_email_for_user, change_email_request, do_email_change_request, confirm_email_change,
SETTING_CHANGE_INITIATED
) )
from student.models import UserProfile, PendingEmailChange from student.models import UserProfile, PendingEmailChange
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
...@@ -19,6 +20,7 @@ from django.conf import settings ...@@ -19,6 +20,7 @@ from django.conf import settings
from edxmako.shortcuts import render_to_string from edxmako.shortcuts import render_to_string
from edxmako.tests import mako_middleware_process_request from edxmako.tests import mako_middleware_process_request
from util.request import safe_get_host from util.request import safe_get_host
from util.testing import EventTestMixin
class TestException(Exception): class TestException(Exception):
...@@ -198,10 +200,11 @@ class ReactivationEmailTests(EmailTestMixin, TestCase): ...@@ -198,10 +200,11 @@ class ReactivationEmailTests(EmailTestMixin, TestCase):
self.assertTrue(response_data['success']) self.assertTrue(response_data['success'])
class EmailChangeRequestTests(TestCase): class EmailChangeRequestTests(EventTestMixin, TestCase):
"""Test changing a user's email address""" """Test changing a user's email address"""
def setUp(self): def setUp(self):
super(EmailChangeRequestTests, self).setUp('student.views.tracker')
self.user = UserFactory.create() self.user = UserFactory.create()
self.new_email = 'new.email@edx.org' self.new_email = 'new.email@edx.org'
self.req_factory = RequestFactory() self.req_factory = RequestFactory()
...@@ -275,6 +278,7 @@ class EmailChangeRequestTests(TestCase): ...@@ -275,6 +278,7 @@ class EmailChangeRequestTests(TestCase):
send_mail.side_effect = [Exception, None] send_mail.side_effect = [Exception, None]
self.request.POST['new_email'] = "valid@email.com" self.request.POST['new_email'] = "valid@email.com"
self.assertFailedRequest(self.run_request(), 'Unable to send email activation link. Please try again later.') self.assertFailedRequest(self.run_request(), 'Unable to send email activation link. Please try again later.')
self.assert_no_events_were_emitted()
@patch('django.core.mail.send_mail') @patch('django.core.mail.send_mail')
@patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
...@@ -295,6 +299,9 @@ class EmailChangeRequestTests(TestCase): ...@@ -295,6 +299,9 @@ class EmailChangeRequestTests(TestCase):
settings.DEFAULT_FROM_EMAIL, settings.DEFAULT_FROM_EMAIL,
[new_email] [new_email]
) )
self.assert_event_emitted(
SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'email', old=old_email, new=new_email
)
@patch('django.contrib.auth.models.User.email_user') @patch('django.contrib.auth.models.User.email_user')
......
...@@ -17,20 +17,22 @@ from django.utils.http import int_to_base36 ...@@ -17,20 +17,22 @@ from django.utils.http import int_to_base36
from mock import Mock, patch from mock import Mock, patch
import ddt import ddt
from student.views import password_reset, password_reset_confirm_wrapper from student.views import password_reset, password_reset_confirm_wrapper, SETTING_CHANGE_INITIATED
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from student.tests.test_email import mock_render_to_string from student.tests.test_email import mock_render_to_string
from util.testing import EventTestMixin
from test_microsite import fake_site_name from test_microsite import fake_site_name
@ddt.ddt @ddt.ddt
class ResetPasswordTests(TestCase): class ResetPasswordTests(EventTestMixin, TestCase):
""" Tests that clicking reset password sends email, and doesn't activate the user """ Tests that clicking reset password sends email, and doesn't activate the user
""" """
request_factory = RequestFactory() request_factory = RequestFactory()
def setUp(self): def setUp(self):
super(ResetPasswordTests, self).setUp('student.views.tracker')
self.user = UserFactory.create() self.user = UserFactory.create()
self.user.is_active = False self.user.is_active = False
self.user.save() self.user.save()
...@@ -55,6 +57,7 @@ class ResetPasswordTests(TestCase): ...@@ -55,6 +57,7 @@ class ResetPasswordTests(TestCase):
'success': True, 'success': True,
'value': "('registration/password_reset_done.html', [])", 'value': "('registration/password_reset_done.html', [])",
}) })
self.assert_no_events_were_emitted()
@patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
def test_nonexist_email_password_reset(self): def test_nonexist_email_password_reset(self):
...@@ -71,6 +74,7 @@ class ResetPasswordTests(TestCase): ...@@ -71,6 +74,7 @@ class ResetPasswordTests(TestCase):
'success': True, 'success': True,
'value': "('registration/password_reset_done.html', [])", 'value': "('registration/password_reset_done.html', [])",
}) })
self.assert_no_events_were_emitted()
@patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
def test_password_reset_ratelimited(self): def test_password_reset_ratelimited(self):
...@@ -88,6 +92,7 @@ class ResetPasswordTests(TestCase): ...@@ -88,6 +92,7 @@ class ResetPasswordTests(TestCase):
bad_req = self.request_factory.post('/password_reset/', {'email': 'thisdoesnotexist@foo.com'}) bad_req = self.request_factory.post('/password_reset/', {'email': 'thisdoesnotexist@foo.com'})
bad_resp = password_reset(bad_req) bad_resp = password_reset(bad_req)
self.assertEquals(bad_resp.status_code, 403) self.assertEquals(bad_resp.status_code, 403)
self.assert_no_events_were_emitted()
cache.clear() cache.clear()
...@@ -98,6 +103,7 @@ class ResetPasswordTests(TestCase): ...@@ -98,6 +103,7 @@ class ResetPasswordTests(TestCase):
"""Tests contents of reset password email, and that user is not active""" """Tests contents of reset password email, and that user is not active"""
good_req = self.request_factory.post('/password_reset/', {'email': self.user.email}) good_req = self.request_factory.post('/password_reset/', {'email': self.user.email})
good_req.user = self.user
good_resp = password_reset(good_req) good_resp = password_reset(good_req)
self.assertEquals(good_resp.status_code, 200) self.assertEquals(good_resp.status_code, 200)
obj = json.loads(good_resp.content) obj = json.loads(good_resp.content)
...@@ -113,6 +119,10 @@ class ResetPasswordTests(TestCase): ...@@ -113,6 +119,10 @@ class ResetPasswordTests(TestCase):
self.assertEquals(len(to_addrs), 1) self.assertEquals(len(to_addrs), 1)
self.assertIn(self.user.email, to_addrs) self.assertIn(self.user.email, to_addrs)
self.assert_event_emitted(
SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None,
)
#test that the user is not active #test that the user is not active
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
self.assertFalse(self.user.is_active) self.assertFalse(self.user.is_active)
...@@ -130,12 +140,17 @@ class ResetPasswordTests(TestCase): ...@@ -130,12 +140,17 @@ class ResetPasswordTests(TestCase):
'/password_reset/', {'email': self.user.email} '/password_reset/', {'email': self.user.email}
) )
req.is_secure = Mock(return_value=is_secure) req.is_secure = Mock(return_value=is_secure)
resp = password_reset(req) req.user = self.user
password_reset(req)
_, msg, _, _ = send_email.call_args[0] _, msg, _, _ = send_email.call_args[0]
expected_msg = "Please go to the following page and choose a new password:\n\n" + protocol expected_msg = "Please go to the following page and choose a new password:\n\n" + protocol
self.assertIn(expected_msg, msg) self.assertIn(expected_msg, msg)
self.assert_event_emitted(
SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None
)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS") @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
@patch('django.core.mail.send_mail') @patch('django.core.mail.send_mail')
@ddt.data(('Crazy Awesome Site', 'Crazy Awesome Site'), (None, 'edX')) @ddt.data(('Crazy Awesome Site', 'Crazy Awesome Site'), (None, 'edX'))
...@@ -150,7 +165,8 @@ class ResetPasswordTests(TestCase): ...@@ -150,7 +165,8 @@ class ResetPasswordTests(TestCase):
'/password_reset/', {'email': self.user.email} '/password_reset/', {'email': self.user.email}
) )
req.get_host = Mock(return_value=domain_override) req.get_host = Mock(return_value=domain_override)
resp = password_reset(req) req.user = self.user
password_reset(req)
_, msg, _, _ = send_email.call_args[0] _, msg, _, _ = send_email.call_args[0]
reset_msg = "you requested a password reset for your user account at {}" reset_msg = "you requested a password reset for your user account at {}"
...@@ -164,6 +180,10 @@ class ResetPasswordTests(TestCase): ...@@ -164,6 +180,10 @@ class ResetPasswordTests(TestCase):
sign_off = "The {} Team".format(platform_name) sign_off = "The {} Team".format(platform_name)
self.assertIn(sign_off, msg) self.assertIn(sign_off, msg)
self.assert_event_emitted(
SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None
)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS") @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
@patch("microsite_configuration.microsite.get_value", fake_site_name) @patch("microsite_configuration.microsite.get_value", fake_site_name)
@patch('django.core.mail.send_mail') @patch('django.core.mail.send_mail')
...@@ -176,13 +196,18 @@ class ResetPasswordTests(TestCase): ...@@ -176,13 +196,18 @@ class ResetPasswordTests(TestCase):
'/password_reset/', {'email': self.user.email} '/password_reset/', {'email': self.user.email}
) )
req.get_host = Mock(return_value=None) req.get_host = Mock(return_value=None)
resp = password_reset(req) req.user = self.user
password_reset(req)
_, msg, _, _ = send_email.call_args[0] _, msg, _, _ = send_email.call_args[0]
reset_msg = "you requested a password reset for your user account at openedx.localhost" reset_msg = "you requested a password reset for your user account at openedx.localhost"
self.assertIn(reset_msg, msg) self.assertIn(reset_msg, msg)
self.assert_event_emitted(
SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None
)
@patch('student.views.password_reset_confirm') @patch('student.views.password_reset_confirm')
def test_reset_password_bad_token(self, reset_confirm): def test_reset_password_bad_token(self, reset_confirm):
"""Tests bad token and uidb36 in password reset""" """Tests bad token and uidb36 in password reset"""
......
...@@ -129,6 +129,8 @@ AUDIT_LOG = logging.getLogger("audit") ...@@ -129,6 +129,8 @@ AUDIT_LOG = logging.getLogger("audit")
ReverifyInfo = namedtuple('ReverifyInfo', 'course_id course_name course_number date status display') # pylint: disable=invalid-name ReverifyInfo = namedtuple('ReverifyInfo', 'course_id course_name course_number date status display') # pylint: disable=invalid-name
SETTING_CHANGE_INITIATED = 'edx.user.settings.change_initiated'
def csrf_token(context): def csrf_token(context):
"""A csrf token that can be included in a form.""" """A csrf token that can be included in a form."""
...@@ -1833,6 +1835,18 @@ def password_reset(request): ...@@ -1833,6 +1835,18 @@ def password_reset(request):
from_email=settings.DEFAULT_FROM_EMAIL, from_email=settings.DEFAULT_FROM_EMAIL,
request=request, request=request,
domain_override=request.get_host()) domain_override=request.get_host())
# When password change is complete, a "edx.user.settings.changed" event will be emitted.
# But because changing the password is multi-step, we also emit an event here so that we can
# track where the request was initiated.
tracker.emit(
SETTING_CHANGE_INITIATED,
{
"setting": "password",
"old": None,
"new": None,
"user_id": request.user.id,
}
)
else: else:
# bad user? tick the rate limiter counter # bad user? tick the rate limiter counter
AUDIT_LOG.info("Bad password_reset user passed in.") AUDIT_LOG.info("Bad password_reset user passed in.")
...@@ -2049,6 +2063,19 @@ def do_email_change_request(user, new_email, activation_key=uuid.uuid4().hex): ...@@ -2049,6 +2063,19 @@ def do_email_change_request(user, new_email, activation_key=uuid.uuid4().hex):
log.error(u'Unable to send email activation link to user from "%s"', from_address, exc_info=True) log.error(u'Unable to send email activation link to user from "%s"', from_address, exc_info=True)
raise ValueError(_('Unable to send email activation link. Please try again later.')) raise ValueError(_('Unable to send email activation link. Please try again later.'))
# When the email address change is complete, a "edx.user.settings.changed" event will be emitted.
# But because changing the email address is multi-step, we also emit an event here so that we can
# track where the request was initiated.
tracker.emit(
SETTING_CHANGE_INITIATED,
{
"setting": "email",
"old": context['old_email'],
"new": context['new_email'],
"user_id": user.id,
}
)
@ensure_csrf_cookie @ensure_csrf_cookie
@transaction.commit_manually @transaction.commit_manually
......
import sys import sys
from mock import patch
from django.conf import settings from django.conf import settings
from django.core.urlresolvers import clear_url_caches, resolve from django.core.urlresolvers import clear_url_caches, resolve
...@@ -55,3 +57,36 @@ class UrlResetMixin(object): ...@@ -55,3 +57,36 @@ class UrlResetMixin(object):
self._reset_urls(urlconf_modules) self._reset_urls(urlconf_modules)
self.addCleanup(lambda: self._reset_urls(urlconf_modules)) self.addCleanup(lambda: self._reset_urls(urlconf_modules))
class EventTestMixin(object):
"""
Generic mixin for verifying that events were emitted during a test.
"""
def setUp(self, tracker):
super(EventTestMixin, self).setUp()
self.tracker = tracker
patcher = patch(self.tracker)
self.mock_tracker = patcher.start()
self.addCleanup(patcher.stop)
def assert_no_events_were_emitted(self):
"""
Ensures no events were emitted since the last event related assertion.
"""
self.assertFalse(self.mock_tracker.emit.called) # pylint: disable=maybe-no-member
def assert_event_emitted(self, event_name, **kwargs):
"""
Verify that an event was emitted with the given parameters.
"""
self.mock_tracker.emit.assert_any_call( # pylint: disable=maybe-no-member
event_name,
kwargs
)
def reset_tracker(self):
"""
Reset the mock tracker in order to forget about old events.
"""
self.mock_tracker.reset_mock()
...@@ -313,13 +313,18 @@ class EventsTestMixin(object): ...@@ -313,13 +313,18 @@ class EventsTestMixin(object):
"time": {"$gt": self.start_time}, "time": {"$gt": self.start_time},
}) })
def verify_events_of_type(self, event_type, expected_events): def verify_events_of_type(self, event_type, expected_events, expected_referers=None):
"""Verify that the expected events of a given type were logged. """Verify that the expected events of a given type were logged.
Args: Args:
event_type (str): The type of event to be verified. event_type (str): The type of event to be verified.
expected_events (list): A list of dicts representing the events that should expected_events (list): A list of dicts representing the events that should
have been fired. have been fired.
expected_referers (list): A list of strings representing the referers for each event
that should been fired (optional). If present, the actual referers compared
with this list, checking that the expected_referers are the suffixes of
actual_referers. For example, if one event is expected, specifying ["/account/settings"]
will verify that the referer for the single event ends with "/account/settings".
""" """
EmptyPromise( EmptyPromise(
lambda: self.get_matching_events(event_type).count() >= len(expected_events), lambda: self.get_matching_events(event_type).count() >= len(expected_events),
...@@ -329,10 +334,22 @@ class EventsTestMixin(object): ...@@ -329,10 +334,22 @@ class EventsTestMixin(object):
# Verify that the correct events were fired # Verify that the correct events were fired
cursor = self.get_matching_events(event_type) cursor = self.get_matching_events(event_type)
actual_events = [] actual_events = []
for i in range(0, cursor.count()): actual_referers = []
raw_event = cursor.next() for __ in range(0, cursor.count()):
actual_events.append(json.loads(raw_event["event"])) emitted_data = cursor.next()
event = emitted_data["event"]
if emitted_data["event_source"] == "browser":
event = json.loads(event)
actual_events.append(event)
actual_referers.append(emitted_data["referer"])
self.assertEqual(expected_events, actual_events) self.assertEqual(expected_events, actual_events)
if expected_referers is not None:
self.assertEqual(len(expected_referers), len(actual_referers), "Number of expected referers is incorrect")
for index, actual_referer in enumerate(actual_referers):
self.assertTrue(
actual_referer.endswith(expected_referers[index]),
"Refer '{0}' does not have correct suffix, '{1}'.".format(actual_referer, expected_referers[index])
)
class UniqueCourseTest(WebAppTest): class UniqueCourseTest(WebAppTest):
......
...@@ -20,7 +20,9 @@ class AccountSettingsTestMixin(EventsTestMixin, WebAppTest): ...@@ -20,7 +20,9 @@ class AccountSettingsTestMixin(EventsTestMixin, WebAppTest):
USERNAME = "test" USERNAME = "test"
PASSWORD = "testpass" PASSWORD = "testpass"
EMAIL = "test@example.com" EMAIL = u"test@example.com"
CHANGE_INITIATED_EVENT_NAME = u"edx.user.settings.change_initiated"
ACCOUNT_SETTINGS_REFERER = u"/account/settings"
def setUp(self): def setUp(self):
""" """
...@@ -218,6 +220,25 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest): ...@@ -218,6 +220,25 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
assert_after_reload=False assert_after_reload=False
) )
self.verify_events_of_type(
self.CHANGE_INITIATED_EVENT_NAME,
[
{
u"user_id": int(self.user_id),
u"setting": u"email",
u"old": self.EMAIL,
u"new": u'me@here.com'
},
{
u"user_id": int(self.user_id),
u"setting": u"email",
u"old": self.EMAIL, # NOTE the first email change was never confirmed, so old has not changed.
u"new": u'you@there.com'
}
],
[self.ACCOUNT_SETTINGS_REFERER, self.ACCOUNT_SETTINGS_REFERER]
)
def test_password_field(self): def test_password_field(self):
""" """
Test behaviour of "Password" field. Test behaviour of "Password" field.
...@@ -229,6 +250,17 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest): ...@@ -229,6 +250,17 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
success_message='Click the link in the message to reset your password.', success_message='Click the link in the message to reset your password.',
) )
self.verify_events_of_type(
self.CHANGE_INITIATED_EVENT_NAME,
[{
u"user_id": int(self.user_id),
u"setting": "password",
u"old": None,
u"new": None
}],
[self.ACCOUNT_SETTINGS_REFERER]
)
@skip( @skip(
'On bokchoy test servers, language changes take a few reloads to fully realize ' 'On bokchoy test servers, language changes take a few reloads to fully realize '
'which means we can no longer reliably match the strings in the html in other tests.' 'which means we can no longer reliably match the strings in the html in other tests.'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment