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]: c42d9628a4/examples/server/server.py
[2]: c42d9628a4/examples/server/static/register.html
[3]: 91453d3639/app/assets/javascripts/updateContent.js (L33)
[4]: https://stackoverflow.com/questions/55971593/navigator-credentials-is-null-on-local-server
[5]: c42d9628a4/fido2/rpid.py (L69)
[6]: https://stackoverflow.com/questions/12394622/does-jquery-ajax-or-load-allow-for-responsetype-arraybuffer
This commit is contained in:
Ben Thorner
2021-05-07 18:10:07 +01:00
parent ebb82b2e80
commit e2cf3e2c70
19 changed files with 531 additions and 14 deletions

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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',

View File

@@ -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'

View File

@@ -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',
}

View File

@@ -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 = `
<a href="#" role="button" draggable="false" class="govuk-button govuk-button--secondary" data-module="register-security-key">
Register a key
</a>`;
</a>`
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()
})
})