Refactor User.webauthn_credentials into a ModelList

This saves a bit of repetition, and lets us attach other methods to the
collection, rather than having multiple methods on the user object
prefixed with the same name, or random functions floating about.
This commit is contained in:
Chris Hill-Scott
2021-06-08 09:41:39 +01:00
parent 9344eabceb
commit f6aa5bdfb8
6 changed files with 56 additions and 42 deletions

View File

@@ -6,8 +6,10 @@ from flask import url_for
from notifications_python_client.errors import HTTPError
from notifications_utils.url_safe_token import generate_token
from app.main.views.user_profile import get_key_from_list_of_keys
from app.models.user import WebAuthnCredential
from app.models.webauthn_credential import (
WebAuthnCredential,
WebAuthnCredentials,
)
from tests.conftest import (
create_api_user_active,
normalize_spaces,
@@ -29,7 +31,7 @@ def test_overview_page_shows_disable_for_platform_admin(
platform_admin_user,
mocker
):
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user')
mocker.patch('app.models.webauthn_credential.WebAuthnCredentials.client_method')
client_request.login(platform_admin_user)
page = client_request.get('main.user_profile')
assert page.select_one('h1').text.strip() == 'Your profile'
@@ -52,7 +54,10 @@ def test_overview_page_shows_security_keys_for_platform_admin(
):
client_request.login(platform_admin_user)
credentials = [webauthn_credential for _ in range(key_count)]
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=credentials)
mocker.patch(
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=credentials,
)
page = client_request.get('main.user_profile')
security_keys_row = page.select_one('#security-keys')
assert ' '.join(security_keys_row.text.split()) == expected_row_text
@@ -369,7 +374,7 @@ def test_should_show_security_keys_page(
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential],
)
@@ -388,9 +393,17 @@ def test_should_show_security_keys_page(
assert register_button.text.strip() == 'Register a key'
def test_get_key_from_list_of_keys(webauthn_credential, webauthn_credential_2):
list_of_keys = [WebAuthnCredential(json) for json in [webauthn_credential, webauthn_credential_2]]
assert get_key_from_list_of_keys(webauthn_credential["id"], list_of_keys) == WebAuthnCredential(webauthn_credential)
def test_get_key_from_list_of_keys(
mocker,
webauthn_credential,
webauthn_credential_2,
fake_uuid,
):
mocker.patch(
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential, webauthn_credential_2],
)
assert WebAuthnCredentials(fake_uuid).by_id(webauthn_credential["id"]) == WebAuthnCredential(webauthn_credential)
def test_should_show_manage_security_key_page(
@@ -402,7 +415,7 @@ def test_should_show_manage_security_key_page(
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential],
)
@@ -425,7 +438,7 @@ def test_manage_security_key_page_404s_when_key_not_found(
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential_2],
)
client_request.get(
@@ -469,7 +482,7 @@ def test_should_redirect_after_change_of_security_key_name(
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential],
)
@@ -502,7 +515,7 @@ def test_user_profile_manage_security_key_should_not_call_api_if_key_name_stays_
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential],
)
@@ -531,7 +544,7 @@ def test_shows_delete_link_for_security_key(
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential],
)
@@ -552,7 +565,7 @@ def test_confirm_delete_security_key(
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential],
)
@@ -601,7 +614,7 @@ def test_delete_security_key_handles_last_credential_error(
):
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential],
)

View File

@@ -50,7 +50,7 @@ def test_begin_register_returns_encoded_options(
platform_admin_client,
webauthn_dev_server,
):
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[])
mocker.patch('app.models.webauthn_credential.WebAuthnCredentials.client_method', return_value=[])
response = platform_admin_client.get(url_for('main.webauthn_begin_register'))
@@ -79,7 +79,7 @@ def test_begin_register_includes_existing_credentials(
mocker,
):
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential, webauthn_credential]
)
@@ -96,7 +96,7 @@ def test_begin_register_stores_state_in_session(
mocker,
):
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[])
response = platform_admin_client.get(
@@ -227,7 +227,7 @@ def test_begin_authentication_returns_encoded_options(client, mocker, webauthn_c
session['user_details'] = {'id': platform_admin_user['id']}
get_creds_mock = mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential]
)
response = client.get(url_for('main.webauthn_begin_authentication'))
@@ -248,7 +248,7 @@ def test_begin_authentication_stores_state_in_session(client, mocker, webauthn_c
session['user_details'] = {'id': platform_admin_user['id']}
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
'app.models.webauthn_credential.WebAuthnCredentials.client_method',
return_value=[webauthn_credential]
)
client.get(url_for('main.webauthn_begin_authentication'))
@@ -268,7 +268,7 @@ 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])
mocker.patch('app.models.webauthn_credential.WebAuthnCredentials.client_method', return_value=[webauthn_credential])
mocker.patch(
'app.main.views.webauthn_credentials._complete_webauthn_login_attempt',
return_value=Mock(location='/foo')
@@ -291,7 +291,7 @@ def test_complete_authentication_403s_if_key_isnt_in_users_credentials(
platform_admin_user['auth_type'] = 'webauthn_auth'
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=[])
mocker.patch('app.models.webauthn_credential.WebAuthnCredentials.client_method', return_value=[])
mock_verify_webauthn_login = mocker.patch('app.main.views.webauthn_credentials._complete_webauthn_login_attempt')
mock_unsuccesful_login_api_call = mocker.patch('app.user_api_client.complete_webauthn_login_attempt')