ensure we reset failed_login_count when appropriate

in verify_user_password, if succesful we reset the failed_login_count.
now we use failed_login_count for 2FA attempts, we need to make sure we
reset it in other places too, so that people don't get blocked,
especially in the reset-password user journey.

* verify_user_code - if it's succesful, reset the failed_login_count
* update_password - reset failed_login_count because either
  * you're logged in and so it's 0 anyway
  * you're resetting your password via pword reset link, and the old
    count isn't relevant anymore
This commit is contained in:
Leo Hemsted
2017-02-16 15:20:30 +00:00
parent d0424e319c
commit ac9739f8a2
4 changed files with 79 additions and 58 deletions

View File

@@ -113,6 +113,8 @@ def reset_failed_login_count(user):
def update_user_password(user, password): def update_user_password(user, password):
# reset failed login count - they've just reset their password so should be fine
user.failed_login_count = 0
user.password = password user.password = password
user.password_changed_at = datetime.utcnow() user.password_changed_at = datetime.utcnow()
db.session.add(user) db.session.add(user)

View File

@@ -130,6 +130,7 @@ def verify_user_code(user_id):
increment_failed_login_count(user_to_verify) increment_failed_login_count(user_to_verify)
raise InvalidRequest("Code has expired", status_code=400) raise InvalidRequest("Code has expired", status_code=400)
use_user_code(code.id) use_user_code(code.id)
reset_failed_login_count(user_to_verify)
return jsonify({}), 204 return jsonify({}), 204

View File

@@ -535,7 +535,6 @@ def test_send_user_confirm_new_email_returns_400_when_email_missing(client, samp
def test_update_user_password_saves_correctly(client, sample_service): def test_update_user_password_saves_correctly(client, sample_service):
assert User.query.count() == 1
sample_user = sample_service.users[0] sample_user = sample_service.users[0]
new_password = '1234567890' new_password = '1234567890'
data = { data = {
@@ -548,7 +547,7 @@ def test_update_user_password_saves_correctly(client, sample_service):
data=json.dumps(data), data=json.dumps(data),
headers=headers) headers=headers)
assert resp.status_code == 200 assert resp.status_code == 200
assert User.query.count() == 1
json_resp = json.loads(resp.get_data(as_text=True)) json_resp = json.loads(resp.get_data(as_text=True))
assert json_resp['data']['password_changed_at'] is not None assert json_resp['data']['password_changed_at'] is not None
data = {'password': new_password} data = {'password': new_password}
@@ -559,3 +558,17 @@ def test_update_user_password_saves_correctly(client, sample_service):
data=json.dumps(data), data=json.dumps(data),
headers=headers) headers=headers)
assert resp.status_code == 204 assert resp.status_code == 204
def test_update_user_password_resets_failed_login_count(client, sample_service):
user = sample_service.users[0]
user.failed_login_count = 1
resp = client.post(
url_for('user.update_password', user_id=user.id),
data=json.dumps({'_password': 'foo'}),
headers=[('Content-Type', 'application/json'), create_authorization_header()]
)
assert resp.status_code == 200
assert user.failed_login_count == 0

View File

@@ -1,27 +1,24 @@
import json import json
import uuid import uuid
import pytest
from datetime import ( from datetime import (
datetime, datetime,
timedelta timedelta
) )
import pytest
from flask import url_for, current_app from flask import url_for, current_app
from freezegun import freeze_time
from app.dao.services_dao import dao_update_service, dao_fetch_service_by_id from app.dao.services_dao import dao_update_service, dao_fetch_service_by_id
from app.models import ( from app.models import (
VerifyCode, VerifyCode,
User, User,
Notification Notification
) )
from app import db from app import db
import app.celery.tasks
from tests import create_authorization_header from tests import create_authorization_header
from freezegun import freeze_time
import app.celery.tasks
def test_user_verify_code(client, def test_user_verify_code(client,
@@ -163,7 +160,7 @@ def test_user_verify_password_missing_password(client,
@pytest.mark.parametrize('research_mode', [True, False]) @pytest.mark.parametrize('research_mode', [True, False])
@freeze_time("2016-01-01 11:09:00.061258") @freeze_time("2016-01-01 11:09:00.061258")
def test_send_user_sms_code(notify_api, def test_send_user_sms_code(client,
sample_user, sample_user,
sms_code_template, sms_code_template,
mocker, mocker,
@@ -171,9 +168,6 @@ def test_send_user_sms_code(notify_api,
""" """
Tests POST endpoint /user/<user_id>/sms-code Tests POST endpoint /user/<user_id>/sms-code
""" """
with notify_api.test_request_context():
with notify_api.test_client() as client:
if research_mode: if research_mode:
notify_service = dao_fetch_service_by_id(current_app.config['NOTIFY_SERVICE_ID']) notify_service = dao_fetch_service_by_id(current_app.config['NOTIFY_SERVICE_ID'])
notify_service.research_mode = True notify_service.research_mode = True
@@ -206,15 +200,13 @@ def test_send_user_sms_code(notify_api,
@freeze_time("2016-01-01 11:09:00.061258") @freeze_time("2016-01-01 11:09:00.061258")
def test_send_user_code_for_sms_with_optional_to_field(notify_api, def test_send_user_code_for_sms_with_optional_to_field(client,
sample_user, sample_user,
sms_code_template, sms_code_template,
mocker): mocker):
""" """
Tests POST endpoint /user/<user_id>/sms-code with optional to field Tests POST endpoint /user/<user_id>/sms-code with optional to field
""" """
with notify_api.test_request_context():
with notify_api.test_client() as client:
to_number = '+441119876757' to_number = '+441119876757'
mocked = mocker.patch('app.user.rest.create_secret_code', return_value='11111') mocked = mocker.patch('app.user.rest.create_secret_code', return_value='11111')
mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
@@ -282,12 +274,11 @@ def test_send_user_email_verification(client,
mocked.assert_called_once_with(([str(notification.id)]), queue="notify") mocked.assert_called_once_with(([str(notification.id)]), queue="notify")
def test_send_email_verification_returns_404_for_bad_input_data(client, notify_db, notify_db_session, mocker): def test_send_email_verification_returns_404_for_bad_input_data(client, notify_db_session, mocker):
""" """
Tests POST endpoint /user/<user_id>/sms-code return 404 for bad input data Tests POST endpoint /user/<user_id>/sms-code return 404 for bad input data
""" """
mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
import uuid
uuid_ = uuid.uuid4() uuid_ = uuid.uuid4()
auth_header = create_authorization_header() auth_header = create_authorization_header()
resp = client.post( resp = client.post(
@@ -297,3 +288,17 @@ def test_send_email_verification_returns_404_for_bad_input_data(client, notify_d
assert resp.status_code == 404 assert resp.status_code == 404
assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found'
assert mocked.call_count == 0 assert mocked.call_count == 0
def test_user_verify_user_code_valid_code_resets_failed_login_count(client, sample_sms_code):
sample_sms_code.failed_login_count = 1
data = json.dumps({
'code_type': sample_sms_code.code_type,
'code': sample_sms_code.txt_code})
resp = client.post(
url_for('user.verify_user_code', user_id=sample_sms_code.user.id),
data=data,
headers=[('Content-Type', 'application/json'), create_authorization_header()])
assert resp.status_code == 204
assert sample_sms_code.user.failed_login_count == 0
assert sample_sms_code.code_used