From 43afcd1064343ac0114da9e9927ab093fb243950 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Wed, 30 Jun 2021 14:58:08 +0100 Subject: [PATCH] Remove redundant restrictions for WebAuthn feature Since the register and authentication APIs work in pairs, we can just put the restrictions on the "begin" API. We weren't testing the restrictions on the "complete" API anyway. For authentication, it's also enough to check if the user has WebAuthn as their auth type, as it's not a big deal if a user continues to login with a security key indefinitely. --- app/main/views/webauthn_credentials.py | 10 ---------- tests/app/main/views/test_webauthn_credentials.py | 11 ----------- 2 files changed, 21 deletions(-) diff --git a/app/main/views/webauthn_credentials.py b/app/main/views/webauthn_credentials.py index 2161edf01..ca2aaad7b 100644 --- a/app/main/views/webauthn_credentials.py +++ b/app/main/views/webauthn_credentials.py @@ -38,7 +38,6 @@ def webauthn_begin_register(): @main.route('/webauthn/register', methods=['POST']) -@user_is_platform_admin def webauthn_complete_register(): if 'webauthn_registration_state' not in session: return cbor.encode("No registration in progress"), 400 @@ -81,9 +80,6 @@ def webauthn_begin_authentication(): if not user_to_login.webauthn_auth: abort(403) - if not user_to_login.platform_admin: - abort(403) - authentication_data, state = current_app.webauthn_server.authenticate_begin( credentials=user_to_login.webauthn_credentials.as_cbor, user_verification="discouraged", # don't ask for PIN @@ -105,12 +101,6 @@ def webauthn_complete_authentication(): user_id = session['user_details']['id'] user_to_login = User.from_id(user_id) - if not user_to_login.webauthn_auth: - abort(403) - - if not user_to_login.platform_admin: - abort(403) - try: _verify_webauthn_authentication(user_to_login) redirect = _complete_webauthn_login_attempt(user_to_login) diff --git a/tests/app/main/views/test_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py index 645ad766e..03fb7855f 100644 --- a/tests/app/main/views/test_webauthn_credentials.py +++ b/tests/app/main/views/test_webauthn_credentials.py @@ -197,17 +197,6 @@ def test_complete_register_handles_missing_state( assert cbor.decode(response.data) == 'No registration in progress' -def test_begin_authentication_forbidden_for_non_platform_admins(client, api_user_active, mock_get_user): - # mock_get_user returns api_user_active so changes to the api user will reflect - api_user_active['auth_type'] = 'webauthn_auth' - - with client.session_transaction() as session: - session['user_details'] = {'id': '1'} - - response = client.get(url_for('main.webauthn_begin_authentication')) - assert response.status_code == 403 - - def test_begin_authentication_forbidden_for_users_without_webauthn(client, mocker, platform_admin_user): platform_admin_user['auth_type'] = 'sms_auth' mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)