mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-20 22:40:31 -04:00
redirect on login; flash errors on failure
the js `fetch` function will follow redirects blindly and return you the final 200 response. when there's an error, we don't want to go anywhere, and we want to use the flask `flash` functionality to pop up an error page (the likely reason for seeing this is using a yubikey that isn't associated with your user). using `flash` and then `window.location.reload()` handles this fine. However, when the user does log in succesfully we need to properly log them in - this includes: * checking their account isn't over the max login count * resetting failed login count to 0 if not * setting a new session id in the database (so other browser windows are logged out) * checking if they need to revalidate their email access (every 90 days) * clearing old user out of the cache This code all happens in the ajax function rather than being in a separate redirect, so that you can't just navigate to the login flow. I wasn't able to unit test that function due how it uses the session and other flask globals, so moved the auth into its own function so it's easy to stub out all that CBOR nonsense. TODO: We still need to pass any `next` URLs through the chain from login page all the way through the javascript AJAX calls and redirects to the log_in_user function
This commit is contained in:
@@ -1,8 +1,10 @@
|
||||
import base64
|
||||
from unittest.mock import ANY
|
||||
|
||||
import pytest
|
||||
from fido2 import cbor
|
||||
from flask import url_for
|
||||
from freezegun.api import freeze_time
|
||||
|
||||
from app.models.webauthn_credential import RegistrationError, WebAuthnCredential
|
||||
|
||||
@@ -49,6 +51,7 @@ def test_begin_register_returns_encoded_options(
|
||||
webauthn_dev_server,
|
||||
):
|
||||
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[])
|
||||
|
||||
response = platform_admin_client.get(url_for('main.webauthn_begin_register'))
|
||||
|
||||
assert response.status_code == 200
|
||||
@@ -257,9 +260,13 @@ def test_complete_authentication_checks_credentials(
|
||||
platform_admin_user['auth_type'] = 'webauthn_auth'
|
||||
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[webauthn_credential])
|
||||
# fake returning a 200 just to keep flask happy, normally this'll redirect
|
||||
mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_login', return_value=('ok', 200))
|
||||
|
||||
response = client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data)
|
||||
assert response.status_code == 302
|
||||
|
||||
# matches response of verify_webauthn_login
|
||||
assert response.data == b'ok'
|
||||
|
||||
|
||||
def test_complete_authentication_403s_if_key_isnt_in_users_credentials(
|
||||
@@ -274,6 +281,7 @@ def test_complete_authentication_403s_if_key_isnt_in_users_credentials(
|
||||
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||
# user has no keys in the database
|
||||
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[])
|
||||
mock_verify_webauthn_login = mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_login')
|
||||
|
||||
response = client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data)
|
||||
assert response.status_code == 403
|
||||
@@ -285,6 +293,8 @@ def test_complete_authentication_403s_if_key_isnt_in_users_credentials(
|
||||
# webauthn state reset so can't replay
|
||||
assert 'webauthn_authentication_state' not in session
|
||||
|
||||
assert mock_verify_webauthn_login.called is False
|
||||
|
||||
|
||||
def test_complete_authentication_clears_session(
|
||||
client,
|
||||
@@ -298,10 +308,90 @@ def test_complete_authentication_clears_session(
|
||||
platform_admin_user['auth_type'] = 'webauthn_auth'
|
||||
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[webauthn_credential])
|
||||
# fake returning a 200 just to keep flask happy, normally this'll redirect
|
||||
mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_login', return_value=('ok', 200))
|
||||
|
||||
response = client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data)
|
||||
assert response.status_code == 302
|
||||
client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data)
|
||||
|
||||
with client.session_transaction() as session:
|
||||
# it's important that we clear the session to ensure that we don't re-use old login artifacts in future
|
||||
assert 'webauthn_authentication_state' not in session
|
||||
|
||||
|
||||
@freeze_time('2020-01-30')
|
||||
def test_verify_webauthn_login_signs_user_in_signs_user_in(client, mocker, mock_create_event, platform_admin_user):
|
||||
platform_admin_user['auth_type'] = 'webauthn_auth'
|
||||
platform_admin_user['email_access_validated_at'] = '2020-01-25T00:00:00.000000Z'
|
||||
|
||||
with client.session_transaction() as session:
|
||||
session['user_details'] = {
|
||||
'id': platform_admin_user['id'],
|
||||
'email': platform_admin_user['email_address']
|
||||
}
|
||||
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||
mocker.patch('app.main.views.webauthn_credentials._complete_webauthn_authentication')
|
||||
mocker.patch('app.user_api_client.verify_webauthn_login', return_value=(True, None))
|
||||
|
||||
resp = client.post(url_for('main.webauthn_complete_authentication'))
|
||||
|
||||
assert resp.status_code == 302
|
||||
assert resp.location == url_for('main.show_accounts_or_dashboard', _external=True)
|
||||
# removes stuff from session
|
||||
with client.session_transaction() as session:
|
||||
assert 'user_details' not in session
|
||||
|
||||
mock_create_event.assert_called_once_with('sucessful_login', ANY)
|
||||
|
||||
|
||||
def test_verify_webauthn_login_signs_user_in_doesnt_sign_user_in_if_api_rejects(
|
||||
client,
|
||||
mocker,
|
||||
platform_admin_user,
|
||||
):
|
||||
platform_admin_user['auth_type'] = 'webauthn_auth'
|
||||
|
||||
with client.session_transaction() as session:
|
||||
session['user_details'] = {
|
||||
'id': platform_admin_user['id'],
|
||||
'email': platform_admin_user['email_address']
|
||||
}
|
||||
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||
mocker.patch('app.main.views.webauthn_credentials._complete_webauthn_authentication')
|
||||
mocker.patch('app.user_api_client.verify_webauthn_login', return_value=(False, None))
|
||||
|
||||
resp = client.post(url_for('main.webauthn_complete_authentication'))
|
||||
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
@freeze_time('2020-04-30')
|
||||
def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed(
|
||||
client,
|
||||
mocker,
|
||||
mock_send_verify_code,
|
||||
platform_admin_user,
|
||||
):
|
||||
platform_admin_user['auth_type'] = 'webauthn_auth'
|
||||
platform_admin_user['email_access_validated_at'] = '2020-01-25T00:00:00.000000Z'
|
||||
user_details = {
|
||||
'id': platform_admin_user['id'],
|
||||
'email': platform_admin_user['email_address']
|
||||
}
|
||||
|
||||
with client.session_transaction() as session:
|
||||
session['user_details'] = user_details
|
||||
|
||||
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||
mocker.patch('app.main.views.webauthn_credentials._complete_webauthn_authentication')
|
||||
mocker.patch('app.user_api_client.verify_webauthn_login', return_value=(True, None))
|
||||
|
||||
resp = client.post(url_for('main.webauthn_complete_authentication'))
|
||||
|
||||
assert resp.status_code == 302
|
||||
assert resp.location == url_for('main.revalidate_email_sent', _external=True)
|
||||
|
||||
with client.session_transaction() as session:
|
||||
# stuff stays in session so we can log them in later when they validate their email
|
||||
assert session['user_details'] == user_details
|
||||
|
||||
mock_send_verify_code.assert_called_once_with(platform_admin_user['id'], 'email', ANY, ANY)
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import uuid
|
||||
from unittest.mock import call
|
||||
from unittest.mock import Mock, call
|
||||
|
||||
import pytest
|
||||
from notifications_python_client.errors import HTTPError
|
||||
|
||||
from app import invite_api_client, service_api_client, user_api_client
|
||||
from app.models.webauthn_credential import WebAuthnCredential
|
||||
@@ -191,6 +192,7 @@ def test_returns_value_from_cache(
|
||||
(user_api_client, 'update_password', [user_id, 'hunter2'], {}),
|
||||
(user_api_client, 'verify_password', [user_id, 'hunter2'], {}),
|
||||
(user_api_client, 'check_verify_code', [user_id, '', ''], {}),
|
||||
(user_api_client, 'complete_webauthn_login_attempt', [user_id], {'is_successful': True}),
|
||||
(user_api_client, 'add_user_to_service', [SERVICE_ONE_ID, user_id, [], []], {}),
|
||||
(user_api_client, 'add_user_to_organisation', [sample_uuid(), user_id], {}),
|
||||
(user_api_client, 'set_user_permissions', [user_id, SERVICE_ONE_ID, []], {}),
|
||||
@@ -263,3 +265,44 @@ def test_create_webauthn_credential_for_user(mocker, webauthn_credential, fake_u
|
||||
|
||||
user_api_client.create_webauthn_credential_for_user(fake_uuid, credential)
|
||||
mock_post.assert_called_once_with(expected_url, data=credential.serialize())
|
||||
|
||||
|
||||
def test_complete_webauthn_login_attempt_returns_true_and_no_message_normally(fake_uuid, mocker):
|
||||
mock_post = mocker.patch('app.notify_client.user_api_client.UserApiClient.post')
|
||||
|
||||
resp = user_api_client.complete_webauthn_login_attempt(fake_uuid, is_successful=True)
|
||||
|
||||
expected_data = {'successful': True}
|
||||
mock_post.assert_called_once_with(f'/user/{fake_uuid}/verify/webauthn-login', data=expected_data)
|
||||
assert resp == (True, '')
|
||||
|
||||
|
||||
def test_complete_webauthn_login_attempt_returns_false_and_message_on_403(fake_uuid, mocker):
|
||||
mock_post = mocker.patch(
|
||||
'app.notify_client.user_api_client.UserApiClient.post',
|
||||
side_effect=HTTPError(
|
||||
response=Mock(
|
||||
status_code=403,
|
||||
json=Mock(
|
||||
return_value={'message': 'forbidden'}
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
resp = user_api_client.complete_webauthn_login_attempt(fake_uuid, is_successful=True)
|
||||
|
||||
expected_data = {'successful': True}
|
||||
mock_post.assert_called_once_with(f'/user/{fake_uuid}/verify/webauthn-login', data=expected_data)
|
||||
|
||||
assert resp == (False, 'forbidden')
|
||||
|
||||
|
||||
def test_complete_webauthn_login_attempt_raises_on_api_error(fake_uuid, mocker):
|
||||
mocker.patch(
|
||||
'app.notify_client.user_api_client.UserApiClient.post',
|
||||
side_effect=HTTPError(response=Mock(status_code=503, message='error'))
|
||||
)
|
||||
|
||||
with pytest.raises(HTTPError):
|
||||
user_api_client.complete_webauthn_login_attempt(fake_uuid, is_successful=True)
|
||||
|
||||
Reference in New Issue
Block a user