From 542b1518755900f698ca6888a13785e302c126f3 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 2 Jun 2021 11:13:53 +0100 Subject: [PATCH] rename verify webauth endpoint to complete it doesn't really do any verification - that's the webauthn code in the browser and the admin app that does that. Instead, this completes the login flow, by marking the user as logged in in the database. Added a docstring that explains this process a bit more, and also added a new route: //complete/webauthn. We'll move the admin code over to use this new url in time --- app/user/rest.py | 14 +++++++++++--- tests/app/user/test_rest.py | 22 ++++++++++++++-------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/app/user/rest.py b/app/user/rest.py index 51595f603..c287a6a75 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -227,11 +227,19 @@ def verify_user_code(user_id): return jsonify({}), 204 +# TODO: Remove the "verify" endpoint once admin no longer points at it +@user_blueprint.route('//complete/webauthn-login', methods=['POST']) @user_blueprint.route('//verify/webauthn-login', methods=['POST']) -def verify_webauthn_login_for_user(user_id): +def complete_login_after_webauthn_authentication_attempt(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. + complete login after a webauthn authentication. There's nothing webauthn specific in this code + but the sms/email flows do this as part of `verify_user_code` above and this is the equivalent spot in the + webauthn flow. + + If the authentication was successful, we've already confirmed the user holds the right security key, + but we still need to check the max login count and set up a current_session_id and last_logged_in_at here. + + If the authentication was unsuccessful then we just bump the failed_login_count in the db. """ data = request.get_json() validate(data, post_verify_webauthn_schema) diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 15b15c734..0e3ac3402 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -1152,14 +1152,14 @@ def test_get_sms_reply_to_for_notify_service(team_member_mobile_edit_template, n @freeze_time('2020-01-01 11:00') -def test_verify_webauthn_login_resets_login_if_succesful(admin_request, sample_user): +def test_complete_login_after_webauthn_authentication_attempt_resets_login_if_successful(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.complete_login_after_webauthn_authentication_attempt', user_id=sample_user.id, _data={'successful': True}, _expected_status=204 @@ -1170,7 +1170,10 @@ def test_verify_webauthn_login_resets_login_if_succesful(admin_request, sample_u 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): +def test_complete_login_after_webauthn_authentication_attempt_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 @@ -1179,7 +1182,7 @@ def test_verify_webauthn_login_returns_204_when_not_successful(admin_request, sa assert sample_user.logged_in_at is None admin_request.post( - 'user.verify_webauthn_login_for_user', + 'user.complete_login_after_webauthn_authentication_attempt', user_id=sample_user.id, _data={'successful': False}, _expected_status=204 @@ -1190,13 +1193,16 @@ def test_verify_webauthn_login_returns_204_when_not_successful(admin_request, sa assert sample_user.logged_in_at is None -def test_verify_webauthn_login_raises_403_if_max_login_count_exceeded(admin_request, sample_user): +def test_complete_login_after_webauthn_authentication_attempt_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.complete_login_after_webauthn_authentication_attempt', user_id=sample_user.id, _data={'successful': True}, _expected_status=403 @@ -1207,9 +1213,9 @@ def test_verify_webauthn_login_raises_403_if_max_login_count_exceeded(admin_requ assert sample_user.logged_in_at is None -def test_verify_webauthn_login_raises_400_if_schema_invalid(admin_request): +def test_complete_login_after_webauthn_authentication_attempt_raises_400_if_schema_invalid(admin_request): admin_request.post( - 'user.verify_webauthn_login_for_user', + 'user.complete_login_after_webauthn_authentication_attempt', user_id=uuid.uuid4(), _data={'successful': 'True'}, _expected_status=400