Add page for security keys with stubbed data

This adds a new platform admin settings row, leading a page which
shows any existing keys and allows a new one to be registered. Until
the APIs for this are implemented, the user API client just returns
some stubbed data for manual testing.

This also includes a basic JavaScript module to do the main work of
registering a new authenticator, to be implemented in the next commits.

Some more minor notes:

- Setting the headings in the mapping_table is necessary to get the
horizontal rule along the top (to match the design).

- Setting caption to False in the mapping_table is necessary to stop
an extra margin appearing at the top.
This commit is contained in:
Ben Thorner
2021-05-07 15:00:01 +01:00
parent e7d6eebdfe
commit ebb82b2e80
13 changed files with 208 additions and 3 deletions

View File

@@ -0,0 +1,14 @@
(function(window) {
"use strict";
window.GOVUK.Modules.RegisterSecurityKey = function() {
this.start = function(component) {
$(component)
.on('click', function(event) {
event.preventDefault();
alert('not implemented');
});
};
};
})(window);

View File

@@ -23,7 +23,11 @@ from app.main.forms import (
TwoFactorForm, TwoFactorForm,
) )
from app.models.user import User from app.models.user import User
from app.utils import user_is_gov_user, user_is_logged_in from app.utils import (
user_is_gov_user,
user_is_logged_in,
user_is_platform_admin,
)
NEW_EMAIL = 'new-email' NEW_EMAIL = 'new-email'
NEW_MOBILE = 'new-mob' NEW_MOBILE = 'new-mob'
@@ -225,3 +229,11 @@ def user_profile_disable_platform_admin_view():
'views/user-profile/disable-platform-admin-view.html', 'views/user-profile/disable-platform-admin-view.html',
form=form form=form
) )
@main.route("/user-profile/security-keys", methods=['GET'])
@user_is_platform_admin
def user_profile_security_keys():
return render_template(
'views/user-profile/security-keys.html',
)

View File

@@ -10,6 +10,7 @@ from app.models.roles_and_permissions import (
all_permissions, all_permissions,
translate_permissions_from_db_to_admin_roles, translate_permissions_from_db_to_admin_roles,
) )
from app.models.webauthn_credential import WebAuthnCredential
from app.notify_client import InviteTokenError from app.notify_client import InviteTokenError
from app.notify_client.invite_api_client import invite_api_client from app.notify_client.invite_api_client import invite_api_client
from app.notify_client.org_invite_api_client import org_invite_api_client from app.notify_client.org_invite_api_client import org_invite_api_client
@@ -343,6 +344,11 @@ class User(JSONModel, UserMixin):
'@nhs.uk', '.nhs.uk', '@nhs.net', '.nhs.net', '@nhs.uk', '.nhs.uk', '@nhs.net', '.nhs.net',
)) ))
@property
def webauthn_credentials(self):
return [WebAuthnCredential(json) for json in
user_api_client.get_webauthn_credentials_for_user(self.id)]
def serialize(self): def serialize(self):
dct = { dct = {
"id": self.id, "id": self.id,

View File

@@ -0,0 +1,11 @@
from app.models import JSONModel
class WebAuthnCredential(JSONModel):
ALLOWED_PROPERTIES = {
'id',
'name',
'credential_data',
'created_at',
'updated_at'
}

View File

@@ -191,5 +191,13 @@ class UserApiClient(NotifyAdminAPIClient):
endpoint = '/user/{}/organisations-and-services'.format(user_id) endpoint = '/user/{}/organisations-and-services'.format(user_id)
return self.get(endpoint) return self.get(endpoint)
def get_webauthn_credentials_for_user(self, user_id):
from datetime import datetime
return [{
'name': 'Ben test',
'created_at': datetime.now().strftime("%Y-%m-%dT%H:%M:%S.%fZ")
}]
user_api_client = UserApiClient() user_api_client = UserApiClient()

View File

@@ -45,6 +45,14 @@
{{ edit_field('Change', url_for('.user_profile_password')) }} {{ edit_field('Change', url_for('.user_profile_password')) }}
{% endcall %} {% endcall %}
{% if current_user.platform_admin %}
{% call row(id='security-keys') %}
{{ text_field('Security keys') }}
{{ text_field(current_user.webauthn_credentials|length) }}
{{ edit_field('Change', url_for('.user_profile_security_keys')) }}
{% endcall %}
{% endif %}
{% if current_user.platform_admin or session.get('disable_platform_admin_view') %} {% if current_user.platform_admin or session.get('disable_platform_admin_view') %}
{% call row() %} {% call row() %}
{{ text_field('Use platform admin view') }} {{ text_field('Use platform admin view') }}

View File

@@ -0,0 +1,58 @@
{% extends "withoutnav_template.html" %}
{% from "components/page-header.html" import page_header %}
{% from "components/button/macro.njk" import govukButton %}
{% from "components/back-link/macro.njk" import govukBackLink %}
{% from "components/table.html" import mapping_table, row, field, row_heading %}
{% set page_title = 'Security keys' %}
{% set credentials = current_user.webauthn_credentials %}
{% block per_page_title %}
{{ page_title }}
{% endblock %}
{% block maincolumn_content %}
{{ page_header(
page_title,
back_link=url_for('.user_profile')
) }}
<div class="govuk-grid-row">
<div class="govuk-grid-column-five-sixths">
{% if credentials %}
{% call mapping_table(
caption=page_title,
field_headings=['Security key'],
field_headings_visible=False,
caption_visible=False,
) %}
{% for credential in credentials %}
{% call row() %}
{% call field() %}
<div class="govuk-body">{{ credential.name }}</div>
<div class="govuk-body hint">Registered {{ credential.created_at|format_delta }}</div>
{% endcall %}
{% endcall %}
{% endfor %}
{% endcall %}
{% else %}
<p class="govuk-body">
Security keys are an alternative way of signing in to Notify.
</p>
{% endif %}
{{ govukButton({
"element": "button",
"text": "Register a key",
"classes": "govuk-button--secondary",
"attributes": {
"data-module": "register-security-key",
}
}) }}
</div>
</div>
{% endblock %}

View File

@@ -178,6 +178,7 @@ const javascripts = () => {
paths.src + 'javascripts/templateFolderForm.js', paths.src + 'javascripts/templateFolderForm.js',
paths.src + 'javascripts/collapsibleCheckboxes.js', paths.src + 'javascripts/collapsibleCheckboxes.js',
paths.src + 'javascripts/radioSlider.js', paths.src + 'javascripts/radioSlider.js',
paths.src + 'javascripts/registerSecurityKey.js',
paths.src + 'javascripts/updateStatus.js', paths.src + 'javascripts/updateStatus.js',
paths.src + 'javascripts/homepage.js', paths.src + 'javascripts/homepage.js',
paths.src + 'javascripts/main.js', paths.src + 'javascripts/main.js',

View File

@@ -11,9 +11,10 @@ from tests.conftest import create_api_user_active, url_for_endpoint_with_token
def test_should_show_overview_page( def test_should_show_overview_page(
client_request, client_request,
): ):
page = client_request.get(('main.user_profile')) page = client_request.get('main.user_profile')
assert page.select_one('h1').text.strip() == 'Your profile' assert page.select_one('h1').text.strip() == 'Your profile'
assert 'Use platform admin view' not in page assert 'Use platform admin view' not in page
assert 'Security keys' not in page
def test_overview_page_shows_disable_for_platform_admin( def test_overview_page_shows_disable_for_platform_admin(
@@ -21,12 +22,28 @@ def test_overview_page_shows_disable_for_platform_admin(
platform_admin_user platform_admin_user
): ):
client_request.login(platform_admin_user) client_request.login(platform_admin_user)
page = client_request.get(('main.user_profile')) page = client_request.get('main.user_profile')
assert page.select_one('h1').text.strip() == 'Your profile' assert page.select_one('h1').text.strip() == 'Your profile'
disable_platform_admin_row = page.select('tr')[-1] disable_platform_admin_row = page.select('tr')[-1]
assert ' '.join(disable_platform_admin_row.text.split()) == 'Use platform admin view Yes Change' assert ' '.join(disable_platform_admin_row.text.split()) == 'Use platform admin view Yes Change'
@pytest.mark.parametrize('has_keys', [False, True])
def test_overview_page_shows_security_keys_for_platform_admin(
mocker,
client_request,
platform_admin_user,
has_keys,
webauthn_credential,
):
client_request.login(platform_admin_user)
credentials = [webauthn_credential] if has_keys else []
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', 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()) == f'Security keys {len(credentials)} Change'
def test_should_show_name_page( def test_should_show_name_page(
client_request client_request
): ):
@@ -320,3 +337,33 @@ def test_can_reenable_platform_admin(client_request, platform_admin_user):
with client_request.session_transaction() as session: with client_request.session_transaction() as session:
assert session['disable_platform_admin_view'] is False assert session['disable_platform_admin_view'] is False
def test_normal_user_doesnt_see_security_keys(client_request):
client_request.get(
'.user_profile_security_keys',
_expected_status=403,
)
def test_should_show_security_keys_page(
mocker,
client_request,
platform_admin_user,
webauthn_credential,
):
client_request.login(platform_admin_user)
mocker.patch(
'app.user_api_client.get_webauthn_credentials_for_user',
return_value=[webauthn_credential],
)
page = client_request.get('.user_profile_security_keys')
assert page.select_one('h1').text.strip() == 'Security keys'
credential_row = page.select('tr')[-1]
assert 'Test credential' in credential_row.text
register_button = page.select_one("[data-module='register-security-key']")
assert register_button.text.strip() == 'Register a key'

View File

@@ -238,3 +238,8 @@ def test_add_user_to_service_calls_correct_endpoint_and_deletes_keys_from_cache(
call('service-{service_id}-template-folders'.format(service_id=service_id)), call('service-{service_id}-template-folders'.format(service_id=service_id)),
call('service-{service_id}'.format(service_id=service_id)), call('service-{service_id}'.format(service_id=service_id)),
] ]
def test_get_webauthn_credentials_for_user_returns_stubbed_data():
credentials = user_api_client.get_webauthn_credentials_for_user('id')
assert credentials[0]['name'] == 'Ben test'

View File

@@ -310,6 +310,7 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, {
'user_profile_mobile_number_confirm', 'user_profile_mobile_number_confirm',
'user_profile_name', 'user_profile_name',
'user_profile_password', 'user_profile_password',
'user_profile_security_keys',
'using_notify', 'using_notify',
'verify', 'verify',
'verify_email', 'verify_email',

View File

@@ -4479,3 +4479,11 @@ def mock_get_invited_org_user_by_id(mocker, sample_org_invite):
'app.org_invite_api_client.get_invited_user', 'app.org_invite_api_client.get_invited_user',
side_effect=_get, side_effect=_get,
) )
@pytest.fixture
def webauthn_credential():
return {
'name': 'Test credential',
'created_at': '2017-10-18T16:57:14.154185Z',
}

View File

@@ -0,0 +1,26 @@
beforeAll(() => {
require('../../app/assets/javascripts/registerSecurityKey.js');
})
afterAll(() => {
require('./support/teardown.js');
})
describe('Register security key', () => {
beforeEach(() => {
document.body.innerHTML = `
<a href="#" role="button" draggable="false" class="govuk-button govuk-button--secondary" data-module="register-security-key">
Register a key
</a>`;
})
test('it is not implemented yet', () => {
window.GOVUK.modules.start();
jest.spyOn(window, 'alert').mockImplementation(() => {});
button = document.querySelector('[data-module="register-security-key"]');
button.click();
expect(window.alert).toBeCalledWith('not implemented')
})
})