From 00b022700791d3afdaf877597d2bf61e1872e119 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 17 May 2021 20:32:01 +0100 Subject: [PATCH 1/2] add endpoint for verifying webauthn login with sms and email auth the api handles verifying logins in the `//verify/code` endpoint, when it checks the code is valid etc. The admin app has already done this for webauthn logins, but we still need an API endpoint so that we can set up the user's db entry to have a new logged in timestamp, a new session id (this is important for logging out other browser sessions), etc. Also, we need to be able to make sure that the user's max login count isn't exceeded. If it's exceeded, we shouldn't let them log in even with a valid webauthn check. This endpoint is a POST where the admin passes in a json dict with key "succesful" being True or False. True sets up the db stuff as mentioned. False just increments the failed login count. --- app/user/rest.py | 27 ++++++++++++++ app/user/users_schema.py | 12 +++++++ tests/app/user/test_rest.py | 71 +++++++++++++++++++++++++++++++++++-- 3 files changed, 107 insertions(+), 3 deletions(-) diff --git a/app/user/rest.py b/app/user/rest.py index f6322aae3..51595f603 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -63,6 +63,7 @@ from app.user.users_schema import ( post_send_user_sms_code_schema, post_set_permissions_schema, post_verify_code_schema, + post_verify_webauthn_schema, ) from app.utils import url_with_token @@ -226,6 +227,32 @@ def verify_user_code(user_id): return jsonify({}), 204 +@user_blueprint.route('//verify/webauthn-login', methods=['POST']) +def verify_webauthn_login_for_user(user_id): + """ + webauthn logins are already verified on the admin app but we still need to + check the max login count and set up a session id etc here. + """ + data = request.get_json() + validate(data, post_verify_webauthn_schema) + + user = get_user_by_id(user_id=user_id) + successful = data['successful'] + + if user.failed_login_count >= current_app.config.get('MAX_VERIFY_CODE_COUNT'): + raise InvalidRequest("Maximum login count exceeded", status_code=403) + + if successful: + user.current_session_id = str(uuid.uuid4()) + user.logged_in_at = datetime.utcnow() + user.failed_login_count = 0 + save_model_user(user) + else: + increment_failed_login_count(user) + + return jsonify({}), 204 + + @user_blueprint.route('//-code', methods=['POST']) def send_user_2fa_code(user_id, code_type): user_to_send_to = get_user_by_id(user_id=user_id) diff --git a/app/user/users_schema.py b/app/user/users_schema.py index a8621c1ab..5ca9b9bf0 100644 --- a/app/user/users_schema.py +++ b/app/user/users_schema.py @@ -11,6 +11,18 @@ post_verify_code_schema = { } +post_verify_webauthn_schema = { + '$schema': 'http://json-schema.org/draft-04/schema#', + 'description': 'POST schema for verifying a webauthn login attempt', + 'type': 'object', + 'properties': { + 'successful': {'type': 'boolean'} + }, + 'required': ['successful'], + 'additionalProperties': False +} + + post_send_user_email_code_schema = { '$schema': 'http://json-schema.org/draft-04/schema#', 'description': ( diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index a6f4dd55a..ec380fbc2 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -1,6 +1,6 @@ +import uuid import json from datetime import datetime -from uuid import UUID import mock import pytest @@ -278,7 +278,7 @@ def test_post_user_attribute(client, sample_user, user_attribute, user_value): }, recipient='newuser@mail.com', reply_to_text='notify@gov.uk', service=mock.ANY, - template_id=UUID('c73f1d71-4049-46d5-a647-d013bdeca3f0'), template_version=1 + template_id=uuid.UUID('c73f1d71-4049-46d5-a647-d013bdeca3f0'), template_version=1 )), ('mobile_number', '+4407700900460', dict( api_key_id=None, key_type='normal', notification_type='sms', @@ -287,7 +287,7 @@ def test_post_user_attribute(client, sample_user, user_attribute, user_value): 'email address': 'notify@digital.cabinet-office.gov.uk' }, recipient='+4407700900460', reply_to_text='testing', service=mock.ANY, - template_id=UUID('8a31520f-4751-4789-8ea1-fe54496725eb'), template_version=1 + template_id=uuid.UUID('8a31520f-4751-4789-8ea1-fe54496725eb'), template_version=1 )) ]) def test_post_user_attribute_with_updated_by( @@ -1149,3 +1149,68 @@ def test_get_sms_reply_to_for_notify_service(team_member_mobile_edit_template, n reply_to = get_sms_reply_to_for_notify_service(number, team_member_mobile_edit_template) assert reply_to == current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER'] \ if expected_reply_to == 'notify_international_sender' else current_app.config['FROM_NUMBER'] + + +@freeze_time('2020-01-01 11:00') +def test_verify_webauthn_login_resets_login_if_succesful(admin_request, sample_user): + sample_user.failed_login_count = 1 + + assert sample_user.current_session_id is None + assert sample_user.logged_in_at is None + + admin_request.post( + 'user.verify_webauthn_login_for_user', + user_id=sample_user.id, + _data={'successful': True}, + _expected_status=204 + ) + + assert sample_user.current_session_id is not None + assert sample_user.failed_login_count == 0 + assert sample_user.logged_in_at == datetime(2020, 1, 1, 11, 0) + + +def test_verify_webauthn_login_returns_204_when_not_successful(admin_request, sample_user): + # when unsuccessful this endpoint is used to bump the failed count. the endpoint still worked + # properly so should return 204 (no content). + sample_user.failed_login_count = 1 + + assert sample_user.current_session_id is None + assert sample_user.logged_in_at is None + + admin_request.post( + 'user.verify_webauthn_login_for_user', + user_id=sample_user.id, + _data={'successful': False}, + _expected_status=204 + ) + + assert sample_user.current_session_id is None + assert sample_user.failed_login_count == 2 + assert sample_user.logged_in_at is None + + +def test_verify_webauthn_login_raises_403_if_max_login_count_exceeded(admin_request, sample_user): + # when unsuccessful this endpoint is used to bump the failed count. the endpoint still worked + # properly so should return 204 (no content). + sample_user.failed_login_count = 10 + + admin_request.post( + 'user.verify_webauthn_login_for_user', + user_id=sample_user.id, + _data={'successful': True}, + _expected_status=403 + ) + + assert sample_user.current_session_id is None + assert sample_user.failed_login_count == 10 + assert sample_user.logged_in_at is None + + +def test_verify_webauthn_login_raises_400_if_schema_invalid(admin_request): + admin_request.post( + 'user.verify_webauthn_login_for_user', + user_id=uuid.uuid4(), + _data={'successful': 'True'}, + _expected_status=400 + ) From 7b5eb5f90562f57787ef3bbe93ec78ffe29693b0 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 19 May 2021 08:21:35 +0100 Subject: [PATCH 2/2] Fix import order check --- tests/app/user/test_rest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index ec380fbc2..15b15c734 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -1,5 +1,5 @@ -import uuid import json +import uuid from datetime import datetime import mock