Stop enumeration of email addresses via forgot pw

https://www.pivotaltracker.com/story/show/113840073

Previously the forgot password page would give an error if you entered an email
address which didn’t belong to an account.

This would allow a potential attacker to know which email addresses were
registered.

This commit changes the response to always be the same, whether or not the email
address exists.

Also, this is a good read about the dangers of asserting whether a mocked method
was called: http://engineeringblog.yelp.com/2015/02/assert_called_once-threat-or-menace.html
This commit is contained in:
Chris Hill-Scott
2016-02-17 10:24:32 +00:00
parent 6a39c8e187
commit e0e445c520
3 changed files with 42 additions and 7 deletions

View File

@@ -15,6 +15,7 @@ def forgot_password():
users_dao.request_password_reset(user)
send_change_password_email(form.email_address.data)
return render_template('views/password-reset-sent.html')
flash('There was an error processing your request')
else:
return render_template('views/password-reset-sent.html')
return render_template('views/forgot-password.html', form=form)

View File

@@ -9,11 +9,13 @@ def test_should_render_forgot_password(app_):
in response.get_data(as_text=True)
def test_should_redirect_to_password_reset_sent_and_state_updated(app_,
api_user_active,
mock_get_user_by_email,
mock_update_user,
mock_send_email):
def test_should_redirect_to_password_reset_sent_and_state_updated(
app_,
api_user_active,
mock_get_user_by_email,
mock_update_user,
mock_send_email
):
with app_.test_request_context():
response = app_.test_client().post(
url_for('.forgot_password'),
@@ -22,3 +24,23 @@ def test_should_redirect_to_password_reset_sent_and_state_updated(app_,
assert (
'You have been sent an email containing a link'
' to reset your password.') in response.get_data(as_text=True)
assert mock_send_email.call_count == 1
def test_should_redirect_to_password_reset_sent_for_non_existant_email_address(
app_,
api_user_active,
mock_dont_get_user_by_email,
mock_update_user,
mock_send_email
):
with app_.test_request_context():
response = app_.test_client().post(
url_for('.forgot_password'),
data={'email_address': 'nope@example.gov.uk'})
assert response.status_code == 200
assert (
'You have been sent an email containing a link'
' to reset your password.') in response.get_data(as_text=True)
mock_dont_get_user_by_email.assert_called_once_with('nope@example.gov.uk')
assert not mock_send_email.called

View File

@@ -41,7 +41,7 @@ def mock_send_sms(request, mocker):
@pytest.fixture(scope='function')
def mock_send_email(request, mocker):
return mocker.patch("app.notifications_api_client.send_email")
return mocker.patch("app.notifications_api_client.send_email", autospec=True)
@pytest.fixture(scope='function')
@@ -287,6 +287,18 @@ def mock_get_user_by_email(mocker, api_user_active):
return mocker.patch('app.user_api_client.get_user_by_email', side_effect=_get_user)
@pytest.fixture(scope='function')
def mock_dont_get_user_by_email(mocker):
def _get_user(email_address):
return None
return mocker.patch(
'app.user_api_client.get_user_by_email',
side_effect=_get_user,
autospec=True
)
@pytest.fixture(scope='function')
def mock_get_user_by_email_request_password_reset(mocker, api_user_request_password_reset):
return mocker.patch(