diff --git a/app/__init__.py b/app/__init__.py index fc9ad0a2d..75fb80518 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -31,7 +31,7 @@ from werkzeug.exceptions import HTTPException as WerkzeugHTTPException from werkzeug.exceptions import abort from werkzeug.local import LocalProxy -from app import proxy_fix +from app import proxy_fix, webauthn_server from app.asset_fingerprinter import asset_fingerprinter from app.commands import setup_commands from app.config import configs @@ -208,6 +208,7 @@ def create_app(application): client.init_app(application) logging.init_app(application) + webauthn_server.init_app(application) login_manager.login_view = 'main.sign_in' login_manager.login_message_category = 'default' diff --git a/app/assets/javascripts/registerSecurityKey.js b/app/assets/javascripts/registerSecurityKey.js new file mode 100644 index 000000000..bd6826b4c --- /dev/null +++ b/app/assets/javascripts/registerSecurityKey.js @@ -0,0 +1,56 @@ +(function(window) { + "use strict"; + + window.GOVUK.Modules.RegisterSecurityKey = function() { + this.start = function(component) { + $(component) + .on('click', function(event) { + event.preventDefault(); + + fetch('/webauthn/register') + .then((response) => { + if (!response.ok) { + throw Error(response.statusText); + } + + return response.arrayBuffer(); + }) + .then((data) => { + var options = window.CBOR.decode(data); + // triggers browser dialogue to select authenticator + return window.navigator.credentials.create(options); + }) + .then((credential) => { + return postWebAuthnCreateResponse( + credential.response, component.data('csrfToken') + ); + }) + .then((response) => { + if (!response.ok) { + throw Error(response.statusText); + } + + window.location.reload(); + }) + .catch((error) => { + console.error(error); + // some browsers will show an error dialogue for some + // errors; to be safe we always pop up an alert + var message = error.message || error; + alert('Error during registration.\n\n' + message); + }); + }); + }; + }; + + function postWebAuthnCreateResponse(response, csrf_token) { + return fetch('/webauthn/register', { + method: 'POST', + headers: { 'X-CSRFToken': csrf_token }, + body: window.CBOR.encode({ + attestationObject: new Uint8Array(response.attestationObject), + clientDataJSON: new Uint8Array(response.clientDataJSON), + }) + }); + } +})(window); diff --git a/app/main/__init__.py b/app/main/__init__.py index 8e0542676..ea2d62f89 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -42,4 +42,5 @@ from app.main.views import ( # noqa isort:skip uploads, user_profile, verify, + webauthn_credentials, ) diff --git a/app/main/views/user_profile.py b/app/main/views/user_profile.py index 9aaff0717..f75624df6 100644 --- a/app/main/views/user_profile.py +++ b/app/main/views/user_profile.py @@ -23,7 +23,11 @@ from app.main.forms import ( TwoFactorForm, ) 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_MOBILE = 'new-mob' @@ -225,3 +229,11 @@ def user_profile_disable_platform_admin_view(): 'views/user-profile/disable-platform-admin-view.html', 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', + ) diff --git a/app/main/views/webauthn_credentials.py b/app/main/views/webauthn_credentials.py new file mode 100644 index 000000000..5304f9d74 --- /dev/null +++ b/app/main/views/webauthn_credentials.py @@ -0,0 +1,46 @@ +from fido2 import cbor +from flask import current_app, request, session +from flask_login import current_user + +from app.main import main +from app.models.webauthn_credential import WebAuthnCredential +from app.notify_client.user_api_client import user_api_client +from app.utils import user_is_platform_admin + + +@main.route('/webauthn/register') +@user_is_platform_admin +def webauthn_begin_register(): + server = current_app.webauthn_server + + registration_data, state = server.register_begin( + { + "id": bytes(current_user.id, 'utf-8'), + "name": current_user.email_address, + "displayName": current_user.name, + }, + credentials=[ + credential.to_credential_data() + for credential in current_user.webauthn_credentials + ], + user_verification="discouraged", # don't ask for PIN + authenticator_attachment="cross-platform", + ) + + session["webauthn_registration_state"] = state + return cbor.encode(registration_data) + + +@main.route('/webauthn/register', methods=['POST']) +@user_is_platform_admin +def webauthn_complete_register(): + credential = WebAuthnCredential.from_registration( + session.pop("webauthn_registration_state"), + cbor.decode(request.get_data()), + ) + + user_api_client.create_webauthn_credential_for_user( + current_user.id, credential + ) + + return '' diff --git a/app/models/user.py b/app/models/user.py index b5888e9c8..7b0c58b78 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -10,6 +10,7 @@ from app.models.roles_and_permissions import ( all_permissions, translate_permissions_from_db_to_admin_roles, ) +from app.models.webauthn_credential import WebAuthnCredential from app.notify_client import InviteTokenError from app.notify_client.invite_api_client import 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', )) + @property + def webauthn_credentials(self): + return [WebAuthnCredential(json) for json in + user_api_client.get_webauthn_credentials_for_user(self.id)] + def serialize(self): dct = { "id": self.id, diff --git a/app/models/webauthn_credential.py b/app/models/webauthn_credential.py new file mode 100644 index 000000000..933d72698 --- /dev/null +++ b/app/models/webauthn_credential.py @@ -0,0 +1,51 @@ +import base64 + +from fido2 import cbor +from fido2.client import ClientData +from fido2.ctap2 import AttestationObject, AttestedCredentialData +from flask import current_app + +from app.models import JSONModel + + +class WebAuthnCredential(JSONModel): + ALLOWED_PROPERTIES = { + 'id', + 'name', + 'credential_data', # contains public key and credential ID for auth + 'registration_response', # sent to API for later auditing (not used) + 'created_at', + 'updated_at' + } + + @classmethod + def from_registration(cls, state, response): + server = current_app.webauthn_server + + auth_data = server.register_complete( + state, + ClientData(response["clientDataJSON"]), + AttestationObject(response["attestationObject"]), + ) + + return cls({ + 'name': 'Unnamed key', + 'credential_data': base64.b64encode( + cbor.encode(auth_data.credential_data), + ), + 'registration_response': base64.b64encode( + cbor.encode(response), + ) + }) + + def to_credential_data(self): + return AttestedCredentialData( + cbor.decode(base64.b64decode(self.credential_data)) + ) + + def serialize(self): + return { + 'name': self.name, + 'credential_data': self.credential_data, + 'registration_response': self.registration_response, + } diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index c5283ca43..09a1974b6 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -191,5 +191,20 @@ class UserApiClient(NotifyAdminAPIClient): endpoint = '/user/{}/organisations-and-services'.format(user_id) return self.get(endpoint) + def get_webauthn_credentials_for_user(self, user_id): + # TODO: remove when using real API + self.credentials = getattr(self, 'credentials', []) + return self.credentials + + def create_webauthn_credential_for_user(self, user_id, credential): + self.credentials = getattr(self, 'credentials', []) + credential_dict = credential.serialize() + + # TODO: remove when using real API + from datetime import datetime + credential_dict['created_at'] = datetime.now().strftime("%Y-%m-%dT%H:%M:%S.%fZ") + + self.credentials += [credential_dict] + user_api_client = UserApiClient() diff --git a/app/templates/views/user-profile.html b/app/templates/views/user-profile.html index edc0b7315..25f60cf2f 100644 --- a/app/templates/views/user-profile.html +++ b/app/templates/views/user-profile.html @@ -45,6 +45,14 @@ {{ edit_field('Change', url_for('.user_profile_password')) }} {% 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') %} {% call row(id='disable-platform-admin') %} {{ text_field('Use platform admin view') }} diff --git a/app/templates/views/user-profile/security-keys.html b/app/templates/views/user-profile/security-keys.html new file mode 100644 index 000000000..625b98ce1 --- /dev/null +++ b/app/templates/views/user-profile/security-keys.html @@ -0,0 +1,59 @@ +{% 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') + ) }} + +
+
+ {% 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() %} +
{{ credential.name }}
+
Registered {{ credential.created_at|format_delta }}
+ {% endcall %} + {% endcall %} + {% endfor %} + {% endcall %} + + {% else %} + +

+ Security keys are an alternative way of signing in to Notify. +

+ + {% endif %} + + {{ govukButton({ + "element": "button", + "text": "Register a key", + "classes": "govuk-button--secondary", + "attributes": { + "data-module": "register-security-key", + "data-csrf-token": csrf_token(), + } + }) }} +
+
+{% endblock %} diff --git a/app/webauthn_server.py b/app/webauthn_server.py new file mode 100644 index 000000000..d8ef00a6e --- /dev/null +++ b/app/webauthn_server.py @@ -0,0 +1,32 @@ +from urllib.parse import urlparse + +from fido2.server import Fido2Server +from fido2.webauthn import PublicKeyCredentialRpEntity + + +def init_app(app): + base_url = urlparse(app.config["ADMIN_BASE_URL"]) + verify_origin_callback = None + + # stub verification in dev (to avoid need for HTTPS) + if app.config["NOTIFY_ENVIRONMENT"] == "development": + verify_origin_callback = stub_origin_checker + + relying_party = PublicKeyCredentialRpEntity( + id=base_url.hostname, + name="GOV.UK Notify", + ) + + app.webauthn_server = Fido2Server( + relying_party, + attestation="direct", + verify_origin=verify_origin_callback, + ) + + # some browsers don't seem to have a default timeout + # 30 seconds seems like a generous amount of time + app.webauthn_server.timeout = 30_000 + + +def stub_origin_checker(*args): + return True diff --git a/gulpfile.js b/gulpfile.js index b803c83f1..88e55fc77 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -148,7 +148,8 @@ const javascripts = () => { paths.npm + 'query-command-supported/dist/queryCommandSupported.min.js', paths.npm + 'diff-dom/diffDOM.js', paths.npm + 'timeago/jquery.timeago.js', - paths.npm + 'textarea-caret/index.js' + paths.npm + 'textarea-caret/index.js', + paths.npm + 'cbor-js/cbor.js' ])); // JS local to this application @@ -178,6 +179,7 @@ const javascripts = () => { paths.src + 'javascripts/templateFolderForm.js', paths.src + 'javascripts/collapsibleCheckboxes.js', paths.src + 'javascripts/radioSlider.js', + paths.src + 'javascripts/registerSecurityKey.js', paths.src + 'javascripts/updateStatus.js', paths.src + 'javascripts/homepage.js', paths.src + 'javascripts/main.js', diff --git a/package.json b/package.json index afab8009c..e062a4ee1 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "dependencies": { "@babel/core": "7.4.0", "@babel/preset-env": "7.4.2", + "cbor-js": "0.1.0", "del": "5.1.0", "diff-dom": "2.5.1", "govuk_frontend_toolkit": "8.1.0", diff --git a/requirements.in b/requirements.in index 794b10ac5..8aa573757 100644 --- a/requirements.in +++ b/requirements.in @@ -20,6 +20,7 @@ eventlet==0.30.2 # pyup: ignore notifications-python-client==6.0.2 Shapely==1.7.1 rtreelib==0.2.0 +fido2==0.9.1 # PaaS awscli-cwlogs>=1.4,<1.5 @@ -28,6 +29,10 @@ itsdangerous==1.1.0 git+https://github.com/alphagov/notifications-utils.git@44.2.0#egg=notifications-utils==44.2.0 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.8-alpha#egg=govuk-frontend-jinja==0.5.8-alpha +# cryptography 3.4+ incorporates Rust code, which isn't supported on PaaS +# e.g. https://github.com/alphagov/notifications-api/pull/3126 +cryptography<3.4 + # gds-metrics requires prometheseus 0.2.0, override that requirement as later versions bring significant performance gains # version 0.10.0 introduced exceptions when workers crashed due to deprecating lower case `prometheus_multiproc_dir`. prometheus-client>=0.9.0,!=0.10.0 diff --git a/requirements.txt b/requirements.txt index b3f999ea1..eff53923b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,12 +29,18 @@ cachetools==4.2.1 # via notifications-utils certifi==2020.12.5 # via requests +cffi==1.14.5 + # via cryptography chardet==4.0.0 # via requests click==7.1.2 # via flask colorama==0.4.3 # via awscli +cryptography==3.3.2 + # via + # -r requirements.in + # fido2 dnspython==1.16.0 # via eventlet docopt==0.6.2 @@ -45,6 +51,8 @@ et-xmlfile==1.0.1 # via openpyxl eventlet==0.30.2 # pyup: ignore # via -r requirements.in +fido2==0.9.1 + # via -r requirements.in flask-login==0.5.0 # via -r requirements.in flask-redis==0.4.0 @@ -124,6 +132,8 @@ prometheus-client==0.10.1 # gds-metrics pyasn1==0.4.8 # via rsa +pycparser==2.20 + # via cffi pyexcel-ezodf==0.3.4 # via pyexcel-ods3 pyexcel-io==0.6.4 @@ -185,7 +195,9 @@ six==1.15.0 # via # awscli-cwlogs # bleach + # cryptography # eventlet + # fido2 # govuk-bank-holidays # python-dateutil smartypants==2.0.1 diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index b1539a2d9..700596559 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -11,9 +11,10 @@ from tests.conftest import create_api_user_active, url_for_endpoint_with_token def test_should_show_overview_page( 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 'Use platform admin view' not in page + assert 'Security keys' not in page 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 ): 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' disable_platform_admin_row = page.select_one('#disable-platform-admin') 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( client_request ): @@ -320,3 +337,33 @@ def test_can_reenable_platform_admin(client_request, platform_admin_user): with client_request.session_transaction() as session: 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' diff --git a/tests/app/main/views/test_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py new file mode 100644 index 000000000..8ae74843a --- /dev/null +++ b/tests/app/main/views/test_webauthn_credentials.py @@ -0,0 +1,126 @@ +import pytest +from fido2 import cbor +from flask import url_for + +from app import webauthn_server + + +@pytest.mark.parametrize('endpoint', [ + 'webauthn_begin_register', +]) +def test_register_forbidden_for_non_platform_admins( + client_request, + endpoint, +): + client_request.get(f'main.{endpoint}', _expected_status=403) + + +def test_begin_register_returns_encoded_options( + app_, + mocker, + platform_admin_user, + platform_admin_client, +): + # override base URL so it's consistent on CI and locally + mocker.patch.dict( + app_.config, + values={'ADMIN_BASE_URL': 'http://localhost:6012'} + ) + webauthn_server.init_app(app_) + + response = platform_admin_client.get( + url_for('main.webauthn_begin_register') + ) + + assert response.status_code == 200 + + webauthn_options = cbor.decode(response.data)['publicKey'] + assert webauthn_options['attestation'] == 'direct' + assert webauthn_options['timeout'] == 30_000 + + auth_selection = webauthn_options['authenticatorSelection'] + assert auth_selection['authenticatorAttachment'] == 'cross-platform' + assert auth_selection['userVerification'] == 'discouraged' + + user_options = webauthn_options['user'] + assert user_options['name'] == platform_admin_user['email_address'] + assert user_options['id'] == bytes(platform_admin_user['id'], 'utf-8') + + relying_party_options = webauthn_options['rp'] + assert relying_party_options['name'] == 'GOV.UK Notify' + assert relying_party_options['id'] == 'localhost' + + +def test_begin_register_includes_existing_credentials( + platform_admin_client, + webauthn_credential, + mocker, +): + mocker.patch( + 'app.user_api_client.get_webauthn_credentials_for_user', + return_value=[webauthn_credential, webauthn_credential] + ) + + response = platform_admin_client.get( + url_for('main.webauthn_begin_register') + ) + + webauthn_options = cbor.decode(response.data)['publicKey'] + assert len(webauthn_options['excludeCredentials']) == 2 + + +def test_begin_register_stores_state_in_session( + platform_admin_client, +): + platform_admin_client.get( + url_for('main.webauthn_begin_register') + ) + + with platform_admin_client.session_transaction() as session: + assert session['webauthn_registration_state'] is not None + + +def test_complete_register_creates_credential( + platform_admin_user, + platform_admin_client, + mocker, +): + with platform_admin_client.session_transaction() as session: + session['webauthn_registration_state'] = 'state' + + user_api_mock = mocker.patch( + 'app.user_api_client.create_webauthn_credential_for_user' + ) + + credential_mock = mocker.patch( + 'app.models.webauthn_credential.WebAuthnCredential.from_registration', + return_value='cred' + ) + + response = platform_admin_client.post( + url_for('main.webauthn_complete_register'), + data=cbor.encode('public_key_credential'), + ) + + assert response.status_code == 200 + credential_mock.assert_called_once_with('state', 'public_key_credential') + user_api_mock.assert_called_once_with(platform_admin_user['id'], 'cred') + + +def test_complete_register_clears_session( + platform_admin_client, + mocker, +): + with platform_admin_client.session_transaction() as session: + session['webauthn_registration_state'] = 'state' + + mocker.patch('app.user_api_client.create_webauthn_credential_for_user') + mocker.patch('app.models.webauthn_credential.WebAuthnCredential.from_registration') + + platform_admin_client.post( + url_for('main.webauthn_complete_register'), + data=cbor.encode('public_key_credential'), + ) + + with platform_admin_client.session_transaction() as session: + assert 'webauthn_registration_state' not in session diff --git a/tests/app/models/test_webauthn_credential.py b/tests/app/models/test_webauthn_credential.py new file mode 100644 index 000000000..4dfd89be7 --- /dev/null +++ b/tests/app/models/test_webauthn_credential.py @@ -0,0 +1,40 @@ +import base64 + +from fido2 import cbor +from fido2.cose import ES256 + +from app import webauthn_server +from app.models.webauthn_credential import WebAuthnCredential + +# noqa adapted from https://github.com/duo-labs/py_webauthn/blob/90e3d97e0182899a35a70fc510280b4082cce19b/tests/test_webauthn.py#L14-L24 +SESSION_STATE = {'challenge': 'bPzpX3hHQtsp9evyKYkaZtVc9UN07PUdJ22vZUdDp94', 'user_verification': 'discouraged'} +CLIENT_DATA_JSON = b'{"type": "webauthn.create", "clientExtensions": {}, "challenge": "bPzpX3hHQtsp9evyKYkaZtVc9UN07PUdJ22vZUdDp94", "origin": "https://webauthn.io"}' # noqa + +# had to use the cbor2 library to re-encode the attestationObject due to implementation differences +ATTESTATION_OBJECT = base64.b64decode(b'o2NmbXRoZmlkby11MmZnYXR0U3RtdKJjc2lnWEgwRgIhAI1qbvWibQos/t3zsTU05IXw1Ek3SDApATok09uc4UBwAiEAv0fB/lgb5Ot3zJ691Vje6iQLAtLhJDiA8zDxaGjcE3hjeDVjgVkCUzCCAk8wggE3oAMCAQICBDxoKU0wDQYJKoZIhvcNAQELBQAwLjEsMCoGA1UEAxMjWXViaWNvIFUyRiBSb290IENBIFNlcmlhbCA0NTcyMDA2MzEwIBcNMTQwODAxMDAwMDAwWhgPMjA1MDA5MDQwMDAwMDBaMDExLzAtBgNVBAMMJll1YmljbyBVMkYgRUUgU2VyaWFsIDIzOTI1NzM0ODExMTE3OTAxMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEvd9nk9t3lMNQMXHtLE1FStlzZnUaSLql2fm1ajoggXlrTt8rzXuSehSTEPvEaEdv/FeSqX22L6Aoa8ajIAIOY6M7MDkwIgYJKwYBBAGCxAoCBBUxLjMuNi4xLjQuMS40MTQ4Mi4xLjUwEwYLKwYBBAGC5RwCAQEEBAMCBSAwDQYJKoZIhvcNAQELBQADggEBAKrADVEJfuwVpIazebzEg0D4Z9OXLs5qZ/ukcONgxkRZ8K04QtP/CB5x6olTlxsj+SXArQDCRzEYUgbws6kZKfuRt2a1P+EzUiqDWLjRILSr+3/o7yR7ZP/GpiFKwdm+czb94POoGD+TS1IYdfXj94mAr5cKWx4EKjh210uovu/pLdLjc8xkQciUrXzZpPR9rT2k/q9HkZhHU+NaCJzky+PTyDbq0KKnzqVhWtfkSBCGw3ezZkTS+5lrvOKbIa24lfeTgu7FST5OwTPCFn8HcfWZMXMSD/KNU+iBqJdAwTLPPDRoLLvPTl29weCAIh+HUpmBQd0UltcPOrA/LFvAf61oYXV0aERhdGFYwnSm6pITyZwvdLIkkrMgz0AmKpTBqVCgOX8pJQtghB7wQQAAAAAAAAAAAAAAAAAAAAAAAAAAAECKU1ppjl9gmhHWyDkgHsUvZmhr6oF3/lD3llzLE2SaOSgOGIsIuAQqgp8JQSUu3r/oOaP8RS44dlQjrH+ALfYtpAECAyYhWCAxnqAfESXOYjKUc2WACuXZ3ch0JHxV0VFrrTyjyjIHXCJYIFnx8H87L4bApR4M+hPcV+fHehEOeW+KCyd0H+WGY8s6') # noqa + + +def test_from_registration_verifies_response(app_, mocker): + mocker.patch.dict( + app_.config, values={ + 'NOTIFY_ENVIRONMENT': 'development', + 'ADMIN_BASE_URL': 'https://webauthn.io', + } + ) + + # disable origin verification for non-HTTPS test + webauthn_server.init_app(app_) + + registration_response = { + 'clientDataJSON': CLIENT_DATA_JSON, + 'attestationObject': ATTESTATION_OBJECT, + } + + credential = WebAuthnCredential.from_registration(SESSION_STATE, registration_response) + assert credential.name == 'Unnamed key' + assert credential.registration_response == base64.b64encode(cbor.encode(registration_response)) + + credential_data = credential.to_credential_data() + assert type(credential_data.credential_id) is bytes + assert type(credential_data.aaguid) is bytes + assert credential_data.public_key[3] == ES256.ALGORITHM diff --git a/tests/app/notify_client/test_user_client.py b/tests/app/notify_client/test_user_client.py index f539b159c..8fab217e8 100644 --- a/tests/app/notify_client/test_user_client.py +++ b/tests/app/notify_client/test_user_client.py @@ -4,6 +4,7 @@ from unittest.mock import call import pytest from app import invite_api_client, service_api_client, user_api_client +from app.models.webauthn_credential import WebAuthnCredential from tests import sample_uuid from tests.conftest import SERVICE_ONE_ID @@ -238,3 +239,14 @@ 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}'.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 len(credentials) == 0 + + +def test_create_webauthn_credential_for_user_stores_stubbed_data(webauthn_credential): + credential = WebAuthnCredential(webauthn_credential) + user_api_client.create_webauthn_credential_for_user('id', credential) + assert len(user_api_client.credentials) == 1 diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 15b25c089..8582733ff 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -310,6 +310,7 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'user_profile_mobile_number_confirm', 'user_profile_name', 'user_profile_password', + 'user_profile_security_keys', 'using_notify', 'verify', 'verify_email', @@ -331,6 +332,8 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'view_template', 'view_template_version', 'view_template_versions', + 'webauthn_begin_register', + 'webauthn_complete_register', 'who_can_use_notify', 'who_its_for', 'write_new_broadcast', diff --git a/tests/app/test_webauthn_server.py b/tests/app/test_webauthn_server.py new file mode 100644 index 000000000..23093378b --- /dev/null +++ b/tests/app/test_webauthn_server.py @@ -0,0 +1,35 @@ +import pytest + +from app import webauthn_server + + +@pytest.mark.parametrize(('environment, allowed'), [ + ('development', True), + ('production', False) +]) +def test_server_origin_verification( + app_, + mocker, + environment, + allowed +): + mocker.patch.dict( + app_.config, + values={'NOTIFY_ENVIRONMENT': environment} + ) + + webauthn_server.init_app(app_) + assert app_.webauthn_server._verify('fake-domain') == allowed + + +def test_server_relying_party_id( + app_, + mocker, +): + mocker.patch.dict( + app_.config, + values={'ADMIN_BASE_URL': 'https://www.notify.works'} + ) + + webauthn_server.init_app(app_) + assert app_.webauthn_server.rp.id == 'www.notify.works' diff --git a/tests/conftest.py b/tests/conftest.py index c20d6d38b..13238e333 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4479,3 +4479,13 @@ def mock_get_invited_org_user_by_id(mocker, sample_org_invite): 'app.org_invite_api_client.get_invited_user', side_effect=_get, ) + + +@pytest.fixture +def webauthn_credential(): + return { + 'name': 'Test credential', + 'credential_data': b'WJ0AAAAAAAAAAAAAAAAAAAAAAECKU1ppjl9gmhHWyDkgHsUvZmhr6oF3/lD3llzLE2SaOSgOGIsIuAQqgp8JQSUu3r/oOaP8RS44dlQjrH+ALfYtpAECAyYhWCAxnqAfESXOYjKUc2WACuXZ3ch0JHxV0VFrrTyjyjIHXCJYIFnx8H87L4bApR4M+hPcV+fHehEOeW+KCyd0H+WGY8s6', # noqa + 'registration_response': 'anything', + 'created_at': '2017-10-18T16:57:14.154185Z', + } diff --git a/tests/javascripts/registerSecurityKey.test.js b/tests/javascripts/registerSecurityKey.test.js new file mode 100644 index 000000000..94e756361 --- /dev/null +++ b/tests/javascripts/registerSecurityKey.test.js @@ -0,0 +1,157 @@ +beforeAll(() => { + window.CBOR = require('../../node_modules/cbor-js/cbor.js') + require('../../app/assets/javascripts/registerSecurityKey.js') + + // disable console.error() so we don't see it in test output + // you might need to comment this out to debug some failures + jest.spyOn(console, 'error').mockImplementation(() => {}) + + // ensure window.alert() is implemented to simplify errors + jest.spyOn(window, 'alert').mockImplementation(() => {}) + + // populate missing values to allow consistent jest.spyOn() + window.fetch = () => {} + window.navigator.credentials = { create: () => {} } +}) + +afterAll(() => { + require('./support/teardown.js') + + // restore window attributes to their original undefined state + delete window.fetch + delete window.navigator.credentials +}) + +describe('Register security key', () => { + let button + + beforeEach(() => { + document.body.innerHTML = ` + + Register a key + ` + + button = document.querySelector('[data-module="register-security-key"]') + window.GOVUK.modules.start() + }) + + test('creates a new credential and reloads', (done) => { + jest.spyOn(window.navigator.credentials, 'create').mockImplementation((options) => { + expect(options).toEqual('options') + + // fake PublicKeyCredential response from WebAuthn API + // both of the nested properties are Array(Buffer) objects + return Promise.resolve({ + response: { + attestationObject: [1, 2, 3], + clientDataJSON: [4, 5, 6], + } + }) + }) + + jest.spyOn(window.location, 'reload').mockImplementation(() => { + // signal that the async promise chain was called + done() + }) + + jest.spyOn(window, 'fetch').mockImplementation((_url, options = {}) => { + // initial fetch of options from the server + if (!options.method) { + // options from the server are CBOR-encoded + webauthnOptions = window.CBOR.encode('options') + + return Promise.resolve({ + ok: true, arrayBuffer: () => webauthnOptions + }) + + // subsequent POST of credential data to server + } else { + decodedData = window.CBOR.decode(options.body) + expect(decodedData.clientDataJSON).toEqual(new Uint8Array([4,5,6])) + expect(decodedData.attestationObject).toEqual(new Uint8Array([1,2,3])) + expect(options.headers['X-CSRFToken']).toBe() + return Promise.resolve({ ok: true }) + } + }) + + button.click() + }) + + test.each([ + ['network'], + ['server'], + ])('alerts if fetching WebAuthn options fails (%s error)', (errorType, done) => { + jest.spyOn(window, 'fetch').mockImplementation((_url, options = {}) => { + if (errorType == 'network') { + return Promise.reject('error') + } else { + return Promise.resolve({ ok: false, statusText: 'error' }) + } + }) + + jest.spyOn(window, 'alert').mockImplementation((msg) => { + expect(msg).toEqual('Error during registration.\n\nerror') + done() + }) + + button.click() + }) + + test.each([ + ['network'], + ['server'], + ])('alerts if sending WebAuthn credentials fails (%s error)', ({errorType}, done) => { + jest.spyOn(window.navigator.credentials, 'create').mockImplementation(() => { + // fake PublicKeyCredential response from WebAuthn API + return Promise.resolve({ response: {} }) + }) + + jest.spyOn(window, 'fetch').mockImplementation((_url, options = {}) => { + // initial fetch of options from the server + if (!options.method) { + webauthnOptions = window.CBOR.encode('options') + + return Promise.resolve({ + ok: true, arrayBuffer: () => webauthnOptions + }) + + // subsequent POST of credential data to server + } else { + if (errorType == 'network') { + return Promise.reject('error') + } else { + return Promise.resolve({ ok: false, statusText: 'error' }) + } + } + }) + + jest.spyOn(window, 'alert').mockImplementation((msg) => { + expect(msg).toEqual('Error during registration.\n\nerror') + done() + }) + + button.click() + }) + + test('alerts if comms with the authenticator fails', (done) => { + jest.spyOn(window.navigator.credentials, 'create').mockImplementation(() => { + return Promise.reject(new DOMException('error')) + }) + + jest.spyOn(window, 'fetch').mockImplementation((_url, options) => { + // initial fetch of options from the server + webauthnOptions = window.CBOR.encode('options') + + return Promise.resolve({ + ok: true, arrayBuffer: () => webauthnOptions + }) + }) + + jest.spyOn(window, 'alert').mockImplementation((msg) => { + expect(msg).toEqual('Error during registration.\n\nerror') + done() + }) + + button.click() + }) +}) diff --git a/tests/javascripts/support/teardown.js b/tests/javascripts/support/teardown.js index c7d00bf8b..a4aa757e0 100644 --- a/tests/javascripts/support/teardown.js +++ b/tests/javascripts/support/teardown.js @@ -2,3 +2,5 @@ window.jQuery = null; $ = null; delete window.GOVUK; + +jest.restoreAllMocks();