From 92b68852245b9a1a85c51e18f673ae3fc5056159 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 10 Jun 2021 19:27:17 +0100 Subject: [PATCH] ensure webauthn page aborts if user isn't allowed --- app/main/views/two_factor.py | 12 ++++-- tests/app/main/views/test_two_factor.py | 51 +++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 96b9b68c7..e4a17d522 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -1,6 +1,7 @@ import json from flask import ( + abort, current_app, redirect, render_template, @@ -91,9 +92,14 @@ def two_factor_sms(): @main.route('/two-factor-webauthn', methods=['GET']) @redirect_to_sign_in def two_factor_webauthn(): - # TODO: Return a sensible error page if the user isn't platform admin or doesn't have webauthn - redirect_url = request.args.get('next') - return render_template('views/two-factor-webauthn.html', redirect_url=redirect_url) + user_id = session['user_details']['id'] + user = User.from_id(user_id) + if not user.platform_admin: + abort(403) + if not user.webauthn_auth: + abort(403) + + return render_template('views/two-factor-webauthn.html') @main.route('/re-validate-email', methods=['GET']) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 50c63e4d0..c25e3b4ff 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -277,6 +277,57 @@ def test_two_factor_get_should_redirect_to_sign_in_if_user_not_in_session( ) +def test_two_factor_webauthn_should_have_auth_signin_button( + client, + platform_admin_user, + mocker, +): + platform_admin_user['auth_type'] = 'webauthn_auth' + mock_get_user = mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) + with client.session_transaction() as session: + session['user_details'] = {'id': platform_admin_user['id'], 'email': platform_admin_user['email_address']} + + response = client.get(url_for('main.two_factor_webauthn')) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + button = page.select_one("button[data-module=authenticate-security-key]") + + assert button.text.strip() == 'Check security key' + + assert button.name == 'button' + mock_get_user.assert_called_once_with(platform_admin_user['id']) + + +def test_two_factor_webauthn_should_reject_non_platform_admins( + client, + api_user_active, + mock_get_user, +): + api_user_active['auth_type'] = 'webauthn_auth' + with client.session_transaction() as session: + session['user_details'] = {'id': api_user_active['id'], 'email': api_user_active['email_address']} + + response = client.get(url_for('main.two_factor_webauthn')) + + assert response.status_code == 403 + + +def test_two_factor_webauthn_should_reject_non_webauthn_auth_users( + client, + platform_admin_user, + mocker, +): + platform_admin_user['auth_type'] = 'sms_auth' + mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) + with client.session_transaction() as session: + session['user_details'] = {'id': platform_admin_user['id'], 'email': platform_admin_user['email_address']} + + response = client.get(url_for('main.two_factor_webauthn')) + + assert response.status_code == 403 + + def test_two_factor_should_activate_pending_user( client, mocker,