From 08f039355311d6b3afc738ae5517241b54c8ce20 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 23 Feb 2022 19:57:27 +0000 Subject: [PATCH] Allow platform admins to change user auth in the UI So we do not have to go into the db when we need to change user auth. We do not allow this for users who use webauthn. We do not want to enable security downgrade for those users. --- app/main/forms.py | 10 ++ app/main/views/find_users.py | 24 +++- app/navigation.py | 1 + app/templates/views/find-users/auth_type.html | 28 +++++ .../views/find-users/user-information.html | 10 ++ tests/app/main/views/test_find_users.py | 103 ++++++++++++++++++ tests/app/test_navigation.py | 1 + 7 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 app/templates/views/find-users/auth_type.html diff --git a/app/main/forms.py b/app/main/forms.py index 2c89859a4..25d9c1077 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1015,6 +1015,16 @@ def filter_by_broadcast_permissions(valuelist): return [entry for entry in valuelist if any(entry in option for option in broadcast_permission_options)] +class AuthTypeForm(StripWhitespaceForm): + auth_type = GovukRadiosField( + 'Sign in using', + choices=[ + ('sms_auth', 'Text message code'), + ('email_auth', 'Email link'), + ] + ) + + class BasePermissionsForm(StripWhitespaceForm): def __init__(self, all_template_folders=None, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/app/main/views/find_users.py b/app/main/views/find_users.py index 3fb817d0b..d3857ee45 100644 --- a/app/main/views/find_users.py +++ b/app/main/views/find_users.py @@ -1,11 +1,11 @@ -from flask import flash, redirect, render_template, request, url_for +from flask import abort, flash, redirect, render_template, request, url_for from flask_login import current_user from notifications_python_client.errors import HTTPError from app import user_api_client from app.event_handlers import create_archive_user_event from app.main import main -from app.main.forms import SearchUsersByEmailForm +from app.main.forms import AuthTypeForm, SearchUsersByEmailForm from app.models.user import User from app.utils.user import user_is_platform_admin @@ -50,3 +50,23 @@ def archive_user(user_id): else: flash('There\'s no way to reverse this! Are you sure you want to archive this user?', 'delete') return user_information(user_id) + + +@main.route("/users//change_auth", methods=['GET', 'POST']) +@user_is_platform_admin +def change_user_auth(user_id): + user = User.from_id(user_id) + if user.webauthn_auth: + abort(403) + + form = AuthTypeForm(auth_type=user.auth_type) + + if form.validate_on_submit(): + user.update(auth_type=form.auth_type.data) + return redirect(url_for('.user_information', user_id=user_id)) + + return render_template( + 'views/find-users/auth_type.html', + form=form, + user=user, + ) diff --git a/app/navigation.py b/app/navigation.py index d528ba855..7a72749c6 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -79,6 +79,7 @@ class HeaderNavigation(Navigation): }, 'platform-admin': { 'archive_user', + 'change_user_auth', 'clear_cache', 'create_email_branding', 'create_letter_branding', diff --git a/app/templates/views/find-users/auth_type.html b/app/templates/views/find-users/auth_type.html new file mode 100644 index 000000000..34e6e1197 --- /dev/null +++ b/app/templates/views/find-users/auth_type.html @@ -0,0 +1,28 @@ +{% extends "views/platform-admin/_base_template.html" %} +{% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} +{% from "components/back-link/macro.njk" import govukBackLink %} + +{% block per_page_title %} + Set auth type for {{ user.name }} +{% endblock %} + +{% block backLink %} + {{ govukBackLink({ "href": url_for('main.user_information', user_id=user.id) }) }} +{% endblock %} + +{% block platform_admin_content %} + +
+
+ {{ page_header('Set auth type for ' + user.name) }} + + {% call form_wrapper() %} + {{ form.auth_type }} + {{ page_footer('Save') }} + {% endcall %} +
+
+ +{% endblock %} diff --git a/app/templates/views/find-users/user-information.html b/app/templates/views/find-users/user-information.html index 536856d09..137e519aa 100644 --- a/app/templates/views/find-users/user-information.html +++ b/app/templates/views/find-users/user-information.html @@ -13,6 +13,7 @@

{{ user.email_address }}

{{ user.mobile_number or 'No mobile number'}}

+

Live services

+ +

Authentication

+

{{ user.auth_type }}

+ {% if user.auth_type != 'webauthn_auth' %} + + Change authentication for this user + + {% endif %} +

Last login

{% if not user.logged_in_at %}

This person has never logged in

diff --git a/tests/app/main/views/test_find_users.py b/tests/app/main/views/test_find_users.py index 4f443c7eb..2c99fd267 100644 --- a/tests/app/main/views/test_find_users.py +++ b/tests/app/main/views/test_find_users.py @@ -120,6 +120,7 @@ def test_user_information_page_shows_information_about_user( ] == [ 'test@gov.uk', '+447700900986', + 'sms_auth', 'Last logged in just now', ] @@ -130,6 +131,7 @@ def test_user_information_page_shows_information_about_user( ] == [ 'Live services', 'Trial mode services', + 'Authentication', 'Last login', ] @@ -141,6 +143,107 @@ def test_user_information_page_shows_information_about_user( ] +def test_user_information_page_shows_change_auth_type_link( + client_request, + platform_admin_user, + api_user_active, + mock_get_organisations_and_services_for_user, + mocker +): + client_request.login(platform_admin_user) + mocker.patch('app.user_api_client.get_user', side_effect=[ + platform_admin_user, + user_json(id_=api_user_active['id'], name="Apple Bloom", auth_type='sms_auth') + ], autospec=True) + + page = client_request.get( + 'main.user_information', user_id=api_user_active['id'] + ) + change_auth_url = url_for('main.change_user_auth', user_id=api_user_active['id']) + + link = page.find('a', {'href': change_auth_url}) + assert normalize_spaces(link.text) == 'Change authentication for this user' + + +def test_user_information_page_doesnt_show_change_auth_type_link_if_user_on_webauthn( + client_request, + platform_admin_user, + api_user_active, + mock_get_organisations_and_services_for_user, + mocker +): + client_request.login(platform_admin_user) + mocker.patch('app.user_api_client.get_user', side_effect=[ + platform_admin_user, + user_json(id_=api_user_active['id'], name="Apple Bloom", auth_type='webauthn_auth') + ], autospec=True) + + page = client_request.get( + 'main.user_information', user_id=api_user_active['id'] + ) + change_auth_url = url_for('main.change_user_auth', user_id=api_user_active['id']) + + link = page.find_all('a', {'href': change_auth_url}) + assert len(link) == 0 + + +@pytest.mark.parametrize('current_auth_type', ['email_auth', 'sms_auth']) +def test_change_user_auth_preselects_current_auth_type( + client_request, + platform_admin_user, + api_user_active, + mocker, + current_auth_type +): + client_request.login(platform_admin_user) + + mocker.patch('app.user_api_client.get_user', side_effect=[ + platform_admin_user, + user_json(id_=api_user_active['id'], name="Apple Bloom", auth_type=current_auth_type) + ], autospec=True) + + checked_radios = client_request.get( + 'main.change_user_auth', + user_id=api_user_active['id'], + ).select( + '.govuk-radios__item input[checked]' + ) + + assert len(checked_radios) == 1 + assert checked_radios[0]['value'] == current_auth_type + + +def test_change_user_auth( + client_request, + platform_admin_user, + api_user_active, + mocker +): + + client_request.login(platform_admin_user) + + mocker.patch('app.user_api_client.get_user', side_effect=[ + platform_admin_user, + user_json(id_=api_user_active['id'], name="Apple Bloom", auth_type='sms_auth') + ], autospec=True) + + mock_update = mocker.patch('app.user_api_client.update_user_attribute') + + client_request.post( + 'main.change_user_auth', + user_id=api_user_active['id'], + _data={ + 'auth_type': 'email_auth' + }, + _expected_redirect=url_for('main.user_information', user_id=api_user_active['id'], _external=True) + ) + + mock_update.assert_called_once_with( + api_user_active['id'], + auth_type='email_auth', + ) + + def test_user_information_page_displays_if_there_are_failed_login_attempts( client_request, platform_admin_user, diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 2cc4058fa..4cb615f34 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -47,6 +47,7 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'cancel_job', 'cancel_letter', 'cancel_letter_job', + 'change_user_auth', 'check_and_resend_text_code', 'check_and_resend_verification_code', 'check_contact_list',