From e2cf3e2c706aa4eb643546e8f67b90632512dc4d Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Fri, 7 May 2021 18:10:07 +0100 Subject: [PATCH] Support registering a new authenticator This adds Yubico's FIDO2 library and two APIs for working with the "navigator.credentials.create()" function in JavaScript. The GET API uses the library to generate options for the "create()" function, and the POST API decodes and verifies the resulting credential. While the options and response are dict-like, CBOR is necessary to encode some of the byte-level values, which can't be represented in JSON. Much of the code here is based on the Yubico library example [1][2]. Implementation notes: - There are definitely better ways to alert the user about failure, but window.alert() will do for the time being. Using location.reload() is also a bit jarring if the page scrolls, but not a major issue. - Ideally we would use window.fetch() to do AJAX calls, but we don't have a polyfill for this, and we use $.ajax() elsewhere [3]. We need to do a few weird tricks [6] to stop jQuery trashing the data. - The FIDO2 server doesn't serve web requests; it's just a "server" in the sense of WebAuthn terminology. It lives in its own module, since it needs to be initialised with the app / config. - $.ajax returns a promise-like object. Although we've used ".fail()" elsewhere [3], I couldn't find a stub object that supports it, so I've gone for ".catch()", and used a Promise stub object in tests. - WebAuthn only works over HTTPS, but there's an exception for "localhost" [4]. However, the library is a bit too strict [5], so we have to disable origin verification to avoid needing HTTPS for dev work. [1]: https://github.com/Yubico/python-fido2/blob/c42d9628a4f33d20c4401096fa8d3fc466d5b77f/examples/server/server.py [2]: https://github.com/Yubico/python-fido2/blob/c42d9628a4f33d20c4401096fa8d3fc466d5b77f/examples/server/static/register.html [3]: https://github.com/alphagov/notifications-admin/blob/91453d36395b7a0cf2998dfb8a5f52cc9e96640f/app/assets/javascripts/updateContent.js#L33 [4]: https://stackoverflow.com/questions/55971593/navigator-credentials-is-null-on-local-server [5]: https://github.com/Yubico/python-fido2/blob/c42d9628a4f33d20c4401096fa8d3fc466d5b77f/fido2/rpid.py#L69 [6]: https://stackoverflow.com/questions/12394622/does-jquery-ajax-or-load-allow-for-responsetype-arraybuffer --- app/__init__.py | 3 +- app/assets/javascripts/registerSecurityKey.js | 59 +++++++- app/main/__init__.py | 1 + app/main/views/webauthn_credentials.py | 43 ++++++ app/models/webauthn_credential.py | 42 +++++- app/notify_client/user_api_client.py | 10 ++ .../views/user-profile/security-keys.html | 1 + app/webauthn_server.py | 32 ++++ gulpfile.js | 3 +- package.json | 1 + requirements.in | 5 + requirements.txt | 12 ++ .../main/views/test_webauthn_credentials.py | 108 ++++++++++++++ tests/app/models/test_webauthn_credential.py | 40 +++++ tests/app/notify_client/test_user_client.py | 7 + tests/app/test_navigation.py | 2 + tests/app/test_webauthn_server.py | 35 +++++ tests/conftest.py | 2 + tests/javascripts/registerSecurityKey.test.js | 139 ++++++++++++++++-- 19 files changed, 531 insertions(+), 14 deletions(-) create mode 100644 app/main/views/webauthn_credentials.py create mode 100644 app/webauthn_server.py create mode 100644 tests/app/main/views/test_webauthn_credentials.py create mode 100644 tests/app/models/test_webauthn_credential.py create mode 100644 tests/app/test_webauthn_server.py 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 index b4678741c..993e325e7 100644 --- a/app/assets/javascripts/registerSecurityKey.js +++ b/app/assets/javascripts/registerSecurityKey.js @@ -3,12 +3,67 @@ window.GOVUK.Modules.RegisterSecurityKey = function() { this.start = function(component) { - $(component) .on('click', function(event) { event.preventDefault(); - alert('not implemented'); + + fetchWebAuthnCreateOptions() + .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(() => { + window.location.reload(); + }) + .catch((error) => { + // there may be other kinds of error we should catch here + // https://github.com/w3c/webauthn/issues/876 + if (error instanceof DOMException) { + console.error(error); + // not all browsers show an error dialogue, so to be safe + // we manually pop one open here (to be improved in future!) + alert('Error communicating with device.\n\n' + error.message); + } else { + // for web requests we need to manually alert the user + // $.ajax seems to log by itself, but that's not visible + alert('Error during registration. Please try again.'); + } + }); }); }; }; + + function fetchWebAuthnCreateOptions() { + var xhrOverride = new XMLHttpRequest(); + xhrOverride.responseType = 'arraybuffer'; + + return $.ajax({ + url: '/webauthn/register', + xhr: () => xhrOverride, + dataType: 'x-binary', + converters: { '* x-binary': (value) => value } + }); + } + + function postWebAuthnCreateResponse(response, csrf_token) { + return $.ajax({ + url: '/webauthn/register', + method: 'POST', + headers: { + 'X-CSRFToken': csrf_token + }, + processData: false, + contentType: 'application/cbor', + data: 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/webauthn_credentials.py b/app/main/views/webauthn_credentials.py new file mode 100644 index 000000000..ad30d9f95 --- /dev/null +++ b/app/main/views/webauthn_credentials.py @@ -0,0 +1,43 @@ +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=[], # TODO: get from user + 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/webauthn_credential.py b/app/models/webauthn_credential.py index 5b40b8148..933d72698 100644 --- a/app/models/webauthn_credential.py +++ b/app/models/webauthn_credential.py @@ -1,3 +1,10 @@ +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 @@ -5,7 +12,40 @@ class WebAuthnCredential(JSONModel): ALLOWED_PROPERTIES = { 'id', 'name', - 'credential_data', + '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 6585a9d2e..cbb2ee74e 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -199,5 +199,15 @@ class UserApiClient(NotifyAdminAPIClient): 'created_at': datetime.now().strftime("%Y-%m-%dT%H:%M:%S.%fZ") }] + 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/security-keys.html b/app/templates/views/user-profile/security-keys.html index 133adfa48..625b98ce1 100644 --- a/app/templates/views/user-profile/security-keys.html +++ b/app/templates/views/user-profile/security-keys.html @@ -51,6 +51,7 @@ "classes": "govuk-button--secondary", "attributes": { "data-module": "register-security-key", + "data-csrf-token": csrf_token(), } }) }} 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 0465d1a1c..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 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 720e6f7d9..c4c5fc7f3 100644 --- a/requirements.in +++ b/requirements.in @@ -20,6 +20,7 @@ eventlet==0.30.2 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 469e0eb1e..e9c133238 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 # 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_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py new file mode 100644 index 000000000..99ed87da5 --- /dev/null +++ b/tests/app/main/views/test_webauthn_credentials.py @@ -0,0 +1,108 @@ +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_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 a54ebc7b0..07aa67a14 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 @@ -243,3 +244,9 @@ def test_add_user_to_service_calls_correct_endpoint_and_deletes_keys_from_cache( 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' + + +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 ea1a46b82..8582733ff 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -332,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 35b5a3785..f7f264ea7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4485,5 +4485,7 @@ def mock_get_invited_org_user_by_id(mocker, sample_org_invite): def webauthn_credential(): return { 'name': 'Test credential', + 'credential_data': 'credential_data', + '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 index 2487fa287..3d265fe6f 100644 --- a/tests/javascripts/registerSecurityKey.test.js +++ b/tests/javascripts/registerSecurityKey.test.js @@ -1,26 +1,147 @@ beforeAll(() => { - require('../../app/assets/javascripts/registerSecurityKey.js'); + 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(() => {}) }) afterAll(() => { - require('./support/teardown.js'); + require('./support/teardown.js') }) 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('it is not implemented yet', () => { - window.GOVUK.modules.start(); - jest.spyOn(window, 'alert').mockImplementation(() => {}); + test('creates a new credential and reloads', (done) => { + // pretend window.navigator.credentials exists in test env + // defineProperty is used as window.navigator is read-only + Object.defineProperty(window.navigator, 'credentials', { + value: { + // fake PublicKeyCredential response from WebAuthn API + // both of the nested properties are Array(Buffer) objects + create: (options) => { + expect(options).toEqual('options') - button = document.querySelector('[data-module="register-security-key"]'); - button.click(); + return Promise.resolve({ + response: { + attestationObject: [1, 2, 3], + clientDataJSON: [4, 5, 6], + } + }) + } + }, + // allow global property to be redefined in other tests + writable: true, + }) - expect(window.alert).toBeCalledWith('not implemented') + // pretend window.location exists in test env + // defineProperty is used as window.location is read-only + Object.defineProperty(window, 'location', { + // signal that the async promise chain was called + value: { reload: () => done() }, + // allow global property to be redefined in other tests + writable: true, + }) + + jest.spyOn(window.$, 'ajax').mockImplementation((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(webauthnOptions) + + // subsequent POST of credential data to server + } else { + decodedData = window.CBOR.decode(options.data) + 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() + } + }) + + button.click() + }) + + test('alerts if fetching WebAuthn options fails', (done) => { + jest.spyOn(window.$, 'ajax').mockImplementation((options) => { + return Promise.reject('error') + }) + + jest.spyOn(window, 'alert').mockImplementation((msg) => { + done() + expect(msg).toEqual('Error during registration. Please try again.') + }) + + button.click() + }) + + test('alerts if sending WebAuthn credentials fails', (done) => { + Object.defineProperty(window.navigator, 'credentials', { + value: { + // fake PublicKeyCredential response from WebAuthn API + create: (options) => { + return Promise.resolve({ response: {} }) + } + }, + // allow global property to be redefined in other tests + writable: true, + }) + + jest.spyOn(window.$, 'ajax').mockImplementation((options) => { + // initial fetch of options from the server + if (!options.method) { + webauthnOptions = window.CBOR.encode('options') + return Promise.resolve(webauthnOptions) + + // subsequent POST of credential data to server + } else { + return Promise.reject('error') + } + }) + + jest.spyOn(window, 'alert').mockImplementation((msg) => { + done() + expect(msg).toEqual('Error during registration. Please try again.') + }) + + button.click() + }) + + test('alerts if comms with the authenticator fails', (done) => { + Object.defineProperty(window.navigator, 'credentials', { + value: { + create: () => { + return Promise.reject(new DOMException('error')) + } + }, + // allow global property to be redefined in other tests + writable: true, + }) + + jest.spyOn(window.$, 'ajax').mockImplementation((options) => { + // initial fetch of options from the server + webauthnOptions = window.CBOR.encode('options') + return Promise.resolve(webauthnOptions) + }) + + jest.spyOn(window, 'alert').mockImplementation((msg) => { + done() + expect(msg).toEqual('Error communicating with device.\n\nerror') + }) + + button.click() }) })