From 5f48367ee54c6ec5b93c60d6879678f2328f1329 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 14 Feb 2017 14:04:11 +0000 Subject: [PATCH 1/6] Set the expiry time on a verify code (2fa) to 10 minutes. When the verify code is wrong or expired increment the failed to login count for the user. When the verify code is successfully used reset the failed login count to 0. --- app/dao/users_dao.py | 2 +- app/user/rest.py | 3 + tests/app/user/test_rest_verify.py | 325 +++++++++++++---------------- 3 files changed, 148 insertions(+), 182 deletions(-) diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 491d05fea..afbd0362c 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -35,7 +35,7 @@ def save_model_user(usr, update_dict={}, pwd=None): def create_user_code(user, code, code_type): verify_code = VerifyCode(code_type=code_type, - expiry_datetime=datetime.utcnow() + timedelta(hours=1), + expiry_datetime=datetime.utcnow() + timedelta(minutes=30), user=user) verify_code.code = code db.session.add(verify_code) diff --git a/app/user/rest.py b/app/user/rest.py index 5f9dd91ed..1e78dc63f 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -123,10 +123,13 @@ def verify_user_code(user_id): code = get_user_code(user_to_verify, txt_code, txt_type) if not code: + increment_failed_login_count(user_to_verify) raise InvalidRequest("Code not found", status_code=404) if datetime.utcnow() > code.expiry_datetime or code.code_used: + increment_failed_login_count(user_to_verify) raise InvalidRequest("Code has expired", status_code=400) use_user_code(code.id) + reset_failed_login_count(user_to_verify) return jsonify({}), 204 diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 6f84d5154..dcd9787aa 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -1,5 +1,4 @@ import json -import moto import pytest from datetime import ( @@ -15,7 +14,7 @@ from app.models import ( Notification ) -from app import db, encryption +from app import db from tests import create_authorization_header from freezegun import freeze_time @@ -23,198 +22,167 @@ from freezegun import freeze_time import app.celery.tasks -def test_user_verify_code_sms(notify_api, - sample_sms_code): - """ - Tests POST endpoint '//verify/code' - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert not VerifyCode.query.first().code_used - data = json.dumps({ - 'code_type': sample_sms_code.code_type, - 'code': sample_sms_code.txt_code}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_code', user_id=sample_sms_code.user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 204 - assert VerifyCode.query.first().code_used +def test_user_verify_code(client, + sample_sms_code): + assert not VerifyCode.query.first().code_used + data = json.dumps({ + 'code_type': sample_sms_code.code_type, + 'code': sample_sms_code.txt_code}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.verify_user_code', user_id=sample_sms_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 204 + assert VerifyCode.query.first().code_used -def test_user_verify_code_sms_missing_code(notify_api, - sample_sms_code): - """ - Tests POST endpoint '//verify/code' - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert not VerifyCode.query.first().code_used - data = json.dumps({'code_type': sample_sms_code.code_type}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_code', user_id=sample_sms_code.user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 400 - assert not VerifyCode.query.first().code_used +def test_user_verify_code_missing_code(client, + sample_sms_code): + assert not VerifyCode.query.first().code_used + data = json.dumps({'code_type': sample_sms_code.code_type}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.verify_user_code', user_id=sample_sms_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 400 + assert not VerifyCode.query.first().code_used + assert User.query.get(sample_sms_code.user.id).failed_login_count == 0 -@moto.mock_sqs -def test_user_verify_code_email(notify_api, - sqs_client_conn, - sample_email_code): - """ - Tests POST endpoint '//verify/code' - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert not VerifyCode.query.first().code_used - data = json.dumps({ - 'code_type': sample_email_code.code_type, - 'code': sample_email_code.txt_code}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_code', user_id=sample_email_code.user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 204 - assert VerifyCode.query.first().code_used +def test_user_verify_code_bad_code_and_increments_failed_login_count(client, + sample_sms_code): + assert not VerifyCode.query.first().code_used + data = json.dumps({ + 'code_type': sample_sms_code.code_type, + 'code': "blah"}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.verify_user_code', user_id=sample_sms_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 404 + assert not VerifyCode.query.first().code_used + assert User.query.get(sample_sms_code.user.id).failed_login_count == 1 -def test_user_verify_code_email_bad_code(notify_api, - sample_email_code): - """ - Tests POST endpoint '//verify/code' - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert not VerifyCode.query.first().code_used - data = json.dumps({ - 'code_type': sample_email_code.code_type, - 'code': "blah"}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_code', user_id=sample_email_code.user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 404 - assert not VerifyCode.query.first().code_used +def test_user_verify_code_resets_failed_login_count_when_successful(client, sample_sms_code): + assert not VerifyCode.query.first().code_used + data = json.dumps({ + 'code_type': sample_sms_code.code_type, + 'code': "blah"}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.verify_user_code', user_id=sample_sms_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 404 + assert not VerifyCode.query.first().code_used + assert User.query.get(sample_sms_code.user.id).failed_login_count == 1 + correct_data = json.dumps({ + 'code_type': sample_sms_code.code_type, + 'code': sample_sms_code.txt_code}) + success_response = client.post( + url_for('user.verify_user_code', user_id=sample_sms_code.user.id), + data=correct_data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert success_response.status_code == 204 + assert VerifyCode.query.first().code_used + assert User.query.get(sample_sms_code.user.id).failed_login_count == 0 -def test_user_verify_code_email_expired_code(notify_api, - sample_email_code): - """ - Tests POST endpoint '//verify/code' - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert not VerifyCode.query.first().code_used - sample_email_code.expiry_datetime = ( - datetime.utcnow() - timedelta(hours=1)) - db.session.add(sample_email_code) - db.session.commit() - data = json.dumps({ - 'code_type': sample_email_code.code_type, - 'code': sample_email_code.txt_code}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_code', user_id=sample_email_code.user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 400 - assert not VerifyCode.query.first().code_used +def test_user_verify_code_expired_code_and_increments_failed_login_count( + client, + sample_sms_code): + assert not VerifyCode.query.first().code_used + sample_sms_code.expiry_datetime = ( + datetime.utcnow() - timedelta(hours=1)) + db.session.add(sample_sms_code) + db.session.commit() + data = json.dumps({ + 'code_type': sample_sms_code.code_type, + 'code': sample_sms_code.txt_code}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.verify_user_code', user_id=sample_sms_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 400 + assert not VerifyCode.query.first().code_used + assert User.query.get(sample_sms_code.user.id).failed_login_count == 1 @freeze_time("2016-01-01 10:00:00.000000") -def test_user_verify_password(notify_api, - notify_db, +def test_user_verify_password(client, notify_db_session, sample_user): - """ - Tests POST endpoint '//verify/password' - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - data = json.dumps({'password': 'password'}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_password', user_id=sample_user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 204 - User.query.get(sample_user.id).logged_in_at == datetime.utcnow() + data = json.dumps({'password': 'password'}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.verify_user_password', user_id=sample_user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 204 + assert User.query.get(sample_user.id).logged_in_at == datetime.utcnow() -def test_user_verify_password_invalid_password(notify_api, +def test_user_verify_password_invalid_password(client, sample_user): - """ - Tests POST endpoint '//verify/password' invalid endpoint. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - data = json.dumps({'password': 'bad password'}) - auth_header = create_authorization_header() + data = json.dumps({'password': 'bad password'}) + auth_header = create_authorization_header() - assert sample_user.failed_login_count == 0 + assert sample_user.failed_login_count == 0 - resp = client.post( - url_for('user.verify_user_password', user_id=sample_user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 400 - json_resp = json.loads(resp.get_data(as_text=True)) - assert 'Incorrect password' in json_resp['message']['password'] - assert sample_user.failed_login_count == 1 + resp = client.post( + url_for('user.verify_user_password', user_id=sample_user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 400 + json_resp = json.loads(resp.get_data(as_text=True)) + assert 'Incorrect password' in json_resp['message']['password'] + assert sample_user.failed_login_count == 1 -def test_user_verify_password_valid_password_resets_failed_logins(notify_api, +def test_user_verify_password_valid_password_resets_failed_logins(client, sample_user): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - data = json.dumps({'password': 'bad password'}) - auth_header = create_authorization_header() + data = json.dumps({'password': 'bad password'}) + auth_header = create_authorization_header() - assert sample_user.failed_login_count == 0 + assert sample_user.failed_login_count == 0 - resp = client.post( - url_for('user.verify_user_password', user_id=sample_user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 400 - json_resp = json.loads(resp.get_data(as_text=True)) - assert 'Incorrect password' in json_resp['message']['password'] + resp = client.post( + url_for('user.verify_user_password', user_id=sample_user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 400 + json_resp = json.loads(resp.get_data(as_text=True)) + assert 'Incorrect password' in json_resp['message']['password'] - assert sample_user.failed_login_count == 1 + assert sample_user.failed_login_count == 1 - data = json.dumps({'password': 'password'}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_password', user_id=sample_user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) + data = json.dumps({'password': 'password'}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.verify_user_password', user_id=sample_user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 204 - assert sample_user.failed_login_count == 0 + assert resp.status_code == 204 + assert sample_user.failed_login_count == 0 -def test_user_verify_password_missing_password(notify_api, +def test_user_verify_password_missing_password(client, sample_user): - """ - Tests POST endpoint '//verify/password' missing password. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - data = json.dumps({'bingo': 'bongo'}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_password', user_id=sample_user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 400 - json_resp = json.loads(resp.get_data(as_text=True)) - assert 'Required field missing data' in json_resp['message']['password'] + data = json.dumps({'bingo': 'bongo'}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.verify_user_password', user_id=sample_user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 400 + json_resp = json.loads(resp.get_data(as_text=True)) + assert 'Required field missing data' in json_resp['message']['password'] @pytest.mark.parametrize('research_mode', [True, False]) @@ -293,22 +261,17 @@ def test_send_user_code_for_sms_with_optional_to_field(notify_api, ) -def test_send_sms_code_returns_404_for_bad_input_data(notify_api, notify_db, notify_db_session): - """ - Tests POST endpoint /user//sms-code return 404 for bad input data - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - data = json.dumps({}) - import uuid - uuid_ = uuid.uuid4() - auth_header = create_authorization_header() - resp = client.post( - url_for('user.send_user_sms_code', user_id=uuid_), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 404 - assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' +def test_send_sms_code_returns_404_for_bad_input_data(client): + data = json.dumps({}) + import uuid + uuid_ = uuid.uuid4() + auth_header = create_authorization_header() + resp = client.post( + url_for('user.send_user_sms_code', user_id=uuid_), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 404 + assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' def test_send_user_email_verification(client, From 6674640330dfcd8319d8ae79aed1d0e3e4fbc6ab Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 15 Feb 2017 10:54:09 +0000 Subject: [PATCH 2/6] Removed resetting of the login count --- app/user/rest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/user/rest.py b/app/user/rest.py index 1e78dc63f..003cc1cd7 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -129,7 +129,6 @@ def verify_user_code(user_id): increment_failed_login_count(user_to_verify) raise InvalidRequest("Code has expired", status_code=400) use_user_code(code.id) - reset_failed_login_count(user_to_verify) return jsonify({}), 204 From 9de88c50baca6d447f4f698b367e3907133eb2f4 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 15 Feb 2017 15:24:42 +0000 Subject: [PATCH 3/6] Remove test for resetting the failed_login_count, the admin app will request that. --- tests/app/user/test_rest_verify.py | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index dcd9787aa..0f5628d7f 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -67,31 +67,6 @@ def test_user_verify_code_bad_code_and_increments_failed_login_count(client, assert User.query.get(sample_sms_code.user.id).failed_login_count == 1 -def test_user_verify_code_resets_failed_login_count_when_successful(client, sample_sms_code): - assert not VerifyCode.query.first().code_used - data = json.dumps({ - 'code_type': sample_sms_code.code_type, - 'code': "blah"}) - auth_header = create_authorization_header() - resp = client.post( - url_for('user.verify_user_code', user_id=sample_sms_code.user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 404 - assert not VerifyCode.query.first().code_used - assert User.query.get(sample_sms_code.user.id).failed_login_count == 1 - correct_data = json.dumps({ - 'code_type': sample_sms_code.code_type, - 'code': sample_sms_code.txt_code}) - success_response = client.post( - url_for('user.verify_user_code', user_id=sample_sms_code.user.id), - data=correct_data, - headers=[('Content-Type', 'application/json'), auth_header]) - assert success_response.status_code == 204 - assert VerifyCode.query.first().code_used - assert User.query.get(sample_sms_code.user.id).failed_login_count == 0 - - def test_user_verify_code_expired_code_and_increments_failed_login_count( client, sample_sms_code): From 52342afe3f2bc020a7a6f79ec8b0ac2a24710af8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 15 Feb 2017 16:18:05 +0000 Subject: [PATCH 4/6] Add a limit to the number of active 2fa codes that we create. At the moment that is set to 10. --- app/config.py | 1 + app/dao/users_dao.py | 8 ++++++++ app/user/rest.py | 7 ++++++- tests/app/dao/test_users_dao.py | 9 +++++++-- tests/app/user/test_rest_verify.py | 22 ++++++++++++++++++++++ 5 files changed, 44 insertions(+), 3 deletions(-) diff --git a/app/config.py b/app/config.py index 45762a33a..10c66a9f3 100644 --- a/app/config.py +++ b/app/config.py @@ -78,6 +78,7 @@ class Config(object): SMS_CHAR_COUNT_LIMIT = 495 BRANDING_PATH = '/images/email-template/crests/' TEST_MESSAGE_FILENAME = 'Test message' + MAX_VERIFY_CODE_COUNT = 10 NOTIFY_SERVICE_ID = 'd6aa2c68-a2d9-4437-ab19-3ae8eb202553' INVITATION_EMAIL_TEMPLATE_ID = '4f46df42-f795-4cc4-83bb-65ca312f49cc' diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index afbd0362c..34360ddc7 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -82,6 +82,14 @@ def delete_user_verify_codes(user): db.session.commit() +def count_user_verify_codes(user): + query = db.session.query( + func.count().label('count') + ).filter(VerifyCode.user == user, + VerifyCode.expiry_datetime <= datetime.utcnow()).one() + return query.count + + def get_user_by_id(user_id=None): if user_id: return User.query.filter_by(id=user_id).one() diff --git a/app/user/rest.py b/app/user/rest.py index 003cc1cd7..9434e80d4 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -12,7 +12,8 @@ from app.dao.users_dao import ( get_user_by_email, create_secret_code, save_user_attribute, - update_user_password + update_user_password, + count_user_verify_codes ) from app.dao.permissions_dao import permission_dao from app.dao.services_dao import dao_fetch_service_by_id @@ -137,6 +138,10 @@ def send_user_sms_code(user_id): user_to_send_to = get_user_by_id(user_id=user_id) verify_code, errors = request_verify_code_schema.load(request.get_json()) + if count_user_verify_codes(user_to_send_to) >= current_app.config.get('MAX_VERIFY_CODE_COUNT'): + # Prevent more than `MAX_VERIFY_CODE_COUNT` active verify codes at a time + return jsonify({}), 204 + secret_code = create_secret_code() create_user_code(user_to_send_to, secret_code, SMS_TYPE) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index e18ef6b79..ed381044e 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -14,8 +14,8 @@ from app.dao.users_dao import ( reset_failed_login_count, get_user_by_email, delete_codes_older_created_more_than_a_day_ago, - update_user_password -) + update_user_password, + count_user_verify_codes) from app.models import User, VerifyCode @@ -140,3 +140,8 @@ def test_update_user_password(notify_api, notify_db, notify_db_session, sample_u assert not sample_user.check_password(password) update_user_password(sample_user, password) assert sample_user.check_password(password) + + +def test_count_user_verify_codes(sample_user): + [make_verify_code(sample_user) for i in range(5)] + assert count_user_verify_codes(sample_user) == 5 diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 0f5628d7f..2a76be285 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -249,6 +249,28 @@ def test_send_sms_code_returns_404_for_bad_input_data(client): assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' +def test_send_sms_code_returns_204_when_too_many_codes_already_created(client, sample_user): + for i in range(10): + verify_code = VerifyCode( + code_type='sms', + _code=12345, + created_at=datetime.utcnow() - timedelta(minutes=10), + expiry_datetime=datetime.utcnow(), + user=sample_user + ) + db.session.add(verify_code) + db.session.commit() + assert VerifyCode.query.count() == 10 + data = json.dumps({}) + auth_header = create_authorization_header() + resp = client.post( + url_for('user.send_user_sms_code', user_id=sample_user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 204 + assert VerifyCode.query.count() == 10 + + def test_send_user_email_verification(client, sample_user, mocker, From ed4b9d34a6e44eea1fe5b2e8c262c705dcb5defd Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 15 Feb 2017 17:41:07 +0000 Subject: [PATCH 5/6] Changes as per code review comments. Fix my backward date math :P --- app/dao/users_dao.py | 7 +++---- app/user/rest.py | 1 + tests/app/dao/test_users_dao.py | 14 ++++++++++---- tests/app/user/test_rest_verify.py | 26 ++++++++++---------------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 34360ddc7..949ad2c7c 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -1,8 +1,6 @@ import random from datetime import (datetime, timedelta) - from sqlalchemy import func - from app import db from app.models import (User, VerifyCode) @@ -48,7 +46,7 @@ def get_user_code(user, code, code_type): # time searching for the correct code. codes = VerifyCode.query.filter_by( user=user, code_type=code_type).order_by( - VerifyCode.created_at.desc()) + VerifyCode.created_at.desc()) retval = None for x in codes: if x.check_code(code): @@ -86,7 +84,8 @@ def count_user_verify_codes(user): query = db.session.query( func.count().label('count') ).filter(VerifyCode.user == user, - VerifyCode.expiry_datetime <= datetime.utcnow()).one() + VerifyCode.expiry_datetime > datetime.utcnow(), + VerifyCode.code_used.is_(False)).one() return query.count diff --git a/app/user/rest.py b/app/user/rest.py index 9434e80d4..ad1c0dee9 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -140,6 +140,7 @@ def send_user_sms_code(user_id): if count_user_verify_codes(user_to_send_to) >= current_app.config.get('MAX_VERIFY_CODE_COUNT'): # Prevent more than `MAX_VERIFY_CODE_COUNT` active verify codes at a time + current_app.logger.warn('Max verify code has exceeded for user {}'.format(user_to_send_to.id)) return jsonify({}), 204 secret_code = create_secret_code() diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index ed381044e..c815eb66b 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -1,5 +1,6 @@ from datetime import datetime, timedelta +from freezegun import freeze_time from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound import pytest @@ -109,13 +110,14 @@ def test_should_not_delete_verification_codes_less_than_one_day_old(sample_user) assert VerifyCode.query.one()._code == "12345" -def make_verify_code(user, age=timedelta(hours=0), code="12335"): +def make_verify_code(user, age=timedelta(hours=0), expiry_age=timedelta(0), code="12335", code_used=False): verify_code = VerifyCode( code_type='sms', _code=code, created_at=datetime.utcnow() - age, - expiry_datetime=datetime.utcnow(), - user=user + expiry_datetime=datetime.utcnow() - expiry_age, + user=user, + code_used=code_used ) db.session.add(verify_code) db.session.commit() @@ -143,5 +145,9 @@ def test_update_user_password(notify_api, notify_db, notify_db_session, sample_u def test_count_user_verify_codes(sample_user): - [make_verify_code(sample_user) for i in range(5)] + with freeze_time(datetime.utcnow() + timedelta(hours=1)): + make_verify_code(sample_user, code_used=True) + make_verify_code(sample_user, expiry_age=timedelta(hours=2)) + [make_verify_code(sample_user) for i in range(5)] + assert count_user_verify_codes(sample_user) == 5 diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 2a76be285..1d9bba17e 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -1,4 +1,6 @@ import json +import uuid + import pytest from datetime import ( @@ -149,11 +151,10 @@ def test_user_verify_password_valid_password_resets_failed_logins(client, def test_user_verify_password_missing_password(client, sample_user): - data = json.dumps({'bingo': 'bongo'}) auth_header = create_authorization_header() resp = client.post( url_for('user.verify_user_password', user_id=sample_user.id), - data=data, + data=json.dumps({'bingo': 'bongo'}), headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 400 json_resp = json.loads(resp.get_data(as_text=True)) @@ -178,14 +179,13 @@ def test_send_user_sms_code(notify_api, notify_service.research_mode = True dao_update_service(notify_service) - data = json.dumps({}) auth_header = create_authorization_header() mocked = mocker.patch('app.user.rest.create_secret_code', return_value='11111') mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') resp = client.post( url_for('user.send_user_sms_code', user_id=sample_user.id), - data=data, + data=json.dumps({}), headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 204 @@ -218,12 +218,11 @@ def test_send_user_code_for_sms_with_optional_to_field(notify_api, to_number = '+441119876757' mocked = mocker.patch('app.user.rest.create_secret_code', return_value='11111') mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - data = json.dumps({'to': to_number}) auth_header = create_authorization_header() resp = client.post( url_for('user.send_user_sms_code', user_id=sample_user.id), - data=data, + data=json.dumps({'to': to_number}), headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 204 @@ -237,13 +236,11 @@ def test_send_user_code_for_sms_with_optional_to_field(notify_api, def test_send_sms_code_returns_404_for_bad_input_data(client): - data = json.dumps({}) - import uuid uuid_ = uuid.uuid4() auth_header = create_authorization_header() resp = client.post( url_for('user.send_user_sms_code', user_id=uuid_), - data=data, + data=json.dumps({}), headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 404 assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' @@ -255,17 +252,16 @@ def test_send_sms_code_returns_204_when_too_many_codes_already_created(client, s code_type='sms', _code=12345, created_at=datetime.utcnow() - timedelta(minutes=10), - expiry_datetime=datetime.utcnow(), + expiry_datetime=datetime.utcnow() + timedelta(minutes=40), user=sample_user ) db.session.add(verify_code) db.session.commit() assert VerifyCode.query.count() == 10 - data = json.dumps({}) auth_header = create_authorization_header() resp = client.post( url_for('user.send_user_sms_code', user_id=sample_user.id), - data=data, + data=json.dumps({}), headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 204 assert VerifyCode.query.count() == 10 @@ -275,12 +271,11 @@ def test_send_user_email_verification(client, sample_user, mocker, email_verification_template): - data = json.dumps({}) mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') auth_header = create_authorization_header() resp = client.post( url_for('user.send_user_email_verification', user_id=str(sample_user.id)), - data=data, + data=json.dumps({}), headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 204 notification = Notification.query.first() @@ -292,13 +287,12 @@ def test_send_email_verification_returns_404_for_bad_input_data(client, notify_d Tests POST endpoint /user//sms-code return 404 for bad input data """ mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - data = json.dumps({}) import uuid uuid_ = uuid.uuid4() auth_header = create_authorization_header() resp = client.post( url_for('user.send_user_email_verification', user_id=uuid_), - data=data, + data=json.dumps({}), headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 404 assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' From b4036e062da282dca126a13e8916fa26f82b51ca Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 16 Feb 2017 12:44:40 +0000 Subject: [PATCH 6/6] rework query to use count() --- app/dao/users_dao.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 949ad2c7c..020830b7c 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -81,12 +81,12 @@ def delete_user_verify_codes(user): def count_user_verify_codes(user): - query = db.session.query( - func.count().label('count') - ).filter(VerifyCode.user == user, - VerifyCode.expiry_datetime > datetime.utcnow(), - VerifyCode.code_used.is_(False)).one() - return query.count + query = VerifyCode.query.filter( + VerifyCode.user == user, + VerifyCode.expiry_datetime > datetime.utcnow(), + VerifyCode.code_used.is_(False) + ) + return query.count() def get_user_by_id(user_id=None):