From bcf2f7dccdbe287ae2334fce5bb639414ca3d8d4 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Fri, 14 May 2021 12:35:22 +0100 Subject: [PATCH 1/3] Fix errorType parameter being null The previous syntax expected the argument would be passed as an object like { errorType: }. --- tests/javascripts/registerSecurityKey.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/javascripts/registerSecurityKey.test.js b/tests/javascripts/registerSecurityKey.test.js index 94e756361..13971ac5f 100644 --- a/tests/javascripts/registerSecurityKey.test.js +++ b/tests/javascripts/registerSecurityKey.test.js @@ -100,7 +100,7 @@ describe('Register security key', () => { test.each([ ['network'], ['server'], - ])('alerts if sending WebAuthn credentials fails (%s error)', ({errorType}, done) => { + ])('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: {} }) From 8502827afbdaf7f91053475a9eed7049e3762195 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Fri, 14 May 2021 09:17:12 +0100 Subject: [PATCH 2/3] Handle errors when registration fails Previously we would raise a 500 error in a variety of cases: - If a second key was being registered simultaneously (e.g. in a separate tab), which means the registration state could be missing after the first registration completes. That smells like an attack. - If the server-side verification failed e.g. origin verification, challenge verification, etc. The library seems to use 'ValueError' for all such errors [1] (after auditing its 'raise' statements, and excluding AttestationError [2], since we're not doing that). - If a key is used that attempts to sign with an unsupported algorithm. This would normally raise a NotImplemented error as part of verifying attestation [3], but we don't do that, so we need to verify the algorithm is supported by the library manually. This adds error handling to return a 400 response and error message in these cases, since the error is not unexpected (i.e. not a 500). A 400 seems more appropriate than a 403, since in many cases it's not clear if the request data is valid. I've used CBOR for the transport encoding, to match the successful request / response encoding. Note that the ordering of then/catch matters in JS - we don't want to catch our own throws! [1]: https://github.com/Yubico/python-fido2/blob/142587b3e698ca0e253c78d75758fda635cac51a/fido2/server.py#L255 [2]: https://github.com/Yubico/python-fido2/blob/c42d9628a4f33d20c4401096fa8d3fc466d5b77f/fido2/attestation/base.py#L39 [3]: https://github.com/Yubico/python-fido2/blob/c42d9628a4f33d20c4401096fa8d3fc466d5b77f/fido2/cose.py#L92 --- app/assets/javascripts/registerSecurityKey.js | 11 +++++- app/main/views/webauthn_credentials.py | 18 ++++++---- app/models/webauthn_credential.py | 21 ++++++++--- .../main/views/test_webauthn_credentials.py | 35 +++++++++++++++++++ tests/app/models/test_webauthn_credential.py | 32 ++++++++++++++++- tests/javascripts/registerSecurityKey.test.js | 20 +++++++---- 6 files changed, 117 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/registerSecurityKey.js b/app/assets/javascripts/registerSecurityKey.js index bd6826b4c..2e3280e40 100644 --- a/app/assets/javascripts/registerSecurityKey.js +++ b/app/assets/javascripts/registerSecurityKey.js @@ -27,7 +27,16 @@ }) .then((response) => { if (!response.ok) { - throw Error(response.statusText); + return response.arrayBuffer() + .then((cbor) => { + return Promise.resolve(window.CBOR.decode(cbor)); + }) + .catch(() => { + throw Error(response.statusText); + }) + .then((text) => { + throw Error(text); + }); } window.location.reload(); diff --git a/app/main/views/webauthn_credentials.py b/app/main/views/webauthn_credentials.py index 5304f9d74..e212374d2 100644 --- a/app/main/views/webauthn_credentials.py +++ b/app/main/views/webauthn_credentials.py @@ -3,7 +3,7 @@ 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.models.webauthn_credential import RegistrationError, WebAuthnCredential from app.notify_client.user_api_client import user_api_client from app.utils import user_is_platform_admin @@ -34,13 +34,19 @@ def webauthn_begin_register(): @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()), - ) + if 'webauthn_registration_state' not in session: + return cbor.encode("No registration in progress"), 400 + + try: + credential = WebAuthnCredential.from_registration( + session.pop("webauthn_registration_state"), + cbor.decode(request.get_data()), + ) + except RegistrationError as e: + return cbor.encode(str(e)), 400 user_api_client.create_webauthn_credential_for_user( current_user.id, credential ) - return '' + return cbor.encode('') diff --git a/app/models/webauthn_credential.py b/app/models/webauthn_credential.py index 97abb9dc1..d9cfdfe67 100644 --- a/app/models/webauthn_credential.py +++ b/app/models/webauthn_credential.py @@ -2,12 +2,17 @@ import base64 from fido2 import cbor from fido2.client import ClientData +from fido2.cose import UnsupportedKey from fido2.ctap2 import AttestationObject, AttestedCredentialData from flask import current_app from app.models import JSONModel +class RegistrationError(Exception): + pass + + class WebAuthnCredential(JSONModel): ALLOWED_PROPERTIES = { 'id', @@ -22,11 +27,17 @@ class WebAuthnCredential(JSONModel): def from_registration(cls, state, response): server = current_app.webauthn_server - auth_data = server.register_complete( - state, - ClientData(response["clientDataJSON"]), - AttestationObject(response["attestationObject"]), - ) + try: + auth_data = server.register_complete( + state, + ClientData(response["clientDataJSON"]), + AttestationObject(response["attestationObject"]), + ) + except ValueError as e: + raise RegistrationError(e) + + if isinstance(auth_data.credential_data.public_key, UnsupportedKey): + raise RegistrationError("Encryption algorithm not supported") return cls({ 'name': 'Unnamed key', diff --git a/tests/app/main/views/test_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py index f41aa62f7..bf96e6d45 100644 --- a/tests/app/main/views/test_webauthn_credentials.py +++ b/tests/app/main/views/test_webauthn_credentials.py @@ -3,6 +3,7 @@ from fido2 import cbor from flask import url_for from app import webauthn_server +from app.models.webauthn_credential import RegistrationError @pytest.mark.parametrize('endpoint', [ @@ -132,3 +133,37 @@ def test_complete_register_clears_session( with platform_admin_client.session_transaction() as session: assert 'webauthn_registration_state' not in session + + +def test_complete_register_handles_library_errors( + platform_admin_client, + mocker, +): + with platform_admin_client.session_transaction() as session: + session['webauthn_registration_state'] = 'state' + + mocker.patch( + 'app.models.webauthn_credential.WebAuthnCredential.from_registration', + side_effect=RegistrationError('error') + ) + + response = platform_admin_client.post( + url_for('main.webauthn_complete_register'), + data=cbor.encode('public_key_credential'), + ) + + assert response.status_code == 400 + assert cbor.decode(response.data) == 'error' + + +def test_complete_register_handles_missing_state( + platform_admin_client, + mocker, +): + response = platform_admin_client.post( + url_for('main.webauthn_complete_register'), + data=cbor.encode('public_key_credential'), + ) + + assert response.status_code == 400 + assert cbor.decode(response.data) == 'No registration in progress' diff --git a/tests/app/models/test_webauthn_credential.py b/tests/app/models/test_webauthn_credential.py index 49e50bab9..44294b8d4 100644 --- a/tests/app/models/test_webauthn_credential.py +++ b/tests/app/models/test_webauthn_credential.py @@ -5,7 +5,7 @@ from fido2 import cbor from fido2.cose import ES256 from app import webauthn_server -from app.models.webauthn_credential import WebAuthnCredential +from app.models.webauthn_credential import RegistrationError, 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'} @@ -14,6 +14,9 @@ CLIENT_DATA_JSON = b'{"type": "webauthn.create", "clientExtensions": {}, "challe # 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 +# manually adapted by working out which character in the encoded CBOR corresponds to the public key algorithm ID +UNSUPPORTED_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+ALfYtpAECAyUhWCAxnqAfESXOYjKUc2WACuXZ3ch0JHxV0VFrrTyjyjIHXCJYIFnx8H87L4bApR4M+hPcV+fHehEOeW+KCyd0H+WGY8s6') # noqa + @pytest.fixture def disable_webauthn_origin_verification(app_, mocker): @@ -56,3 +59,30 @@ def test_from_registration_encodes_as_unicode(disable_webauthn_origin_verificati assert type(serialized_credential['credential_data']) == str assert type(serialized_credential['registration_response']) == str + + +def test_from_registration_handles_library_errors(app_): + # enable origin verification for non-HTTPS test + webauthn_server.init_app(app_) + + registration_response = { + 'clientDataJSON': CLIENT_DATA_JSON, + 'attestationObject': ATTESTATION_OBJECT, + } + + with pytest.raises(RegistrationError) as exc_info: + WebAuthnCredential.from_registration(SESSION_STATE, registration_response) + + assert 'Invalid origin' in str(exc_info.value) + + +def test_from_registration_handles_unsupported_keys(disable_webauthn_origin_verification): + registration_response = { + 'clientDataJSON': CLIENT_DATA_JSON, + 'attestationObject': UNSUPPORTED_ATTESTATION_OBJECT, + } + + with pytest.raises(RegistrationError) as exc_info: + WebAuthnCredential.from_registration(SESSION_STATE, registration_response) + + assert 'Encryption algorithm not supported' in str(exc_info.value) diff --git a/tests/javascripts/registerSecurityKey.test.js b/tests/javascripts/registerSecurityKey.test.js index 13971ac5f..53e13e9d3 100644 --- a/tests/javascripts/registerSecurityKey.test.js +++ b/tests/javascripts/registerSecurityKey.test.js @@ -98,9 +98,10 @@ describe('Register security key', () => { }) test.each([ - ['network'], - ['server'], - ])('alerts if sending WebAuthn credentials fails (%s error)', (errorType, done) => { + ['network error'], + ['internal server error'], + ['bad request'], + ])('alerts if sending WebAuthn credentials fails (%s)', (errorType, done) => { jest.spyOn(window.navigator.credentials, 'create').mockImplementation(() => { // fake PublicKeyCredential response from WebAuthn API return Promise.resolve({ response: {} }) @@ -117,10 +118,15 @@ describe('Register security key', () => { // subsequent POST of credential data to server } else { - if (errorType == 'network') { - return Promise.reject('error') - } else { - return Promise.resolve({ ok: false, statusText: 'error' }) + switch (errorType) { + case 'network error': + return Promise.reject('error') + case 'bad request': + message = Promise.resolve(window.CBOR.encode('error')) + return Promise.resolve({ ok: false, arrayBuffer: () => message }) + case 'internal server error': + message = Promise.reject('encoding error') + return Promise.resolve({ ok: false, arrayBuffer: () => message, statusText: 'error' }) } } }) From fd6329b92efee8f351dd445d3984f7c3e6f5e522 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 17 May 2021 11:37:47 +0100 Subject: [PATCH 3/3] Fix app config leaking between tests We need to re-initialise the webauthn_server module with original app config, since this state is global across all tests. Since the behaviour of the original fixture wasn't specific to verifying the origin, I've renamed the fixture as part of making it global. In order to keep the fixture simple, I've rewritten the test for the webauthn_server module, so they don't touch the app fixture. --- .../main/views/test_webauthn_credentials.py | 15 ++------ tests/app/models/test_webauthn_credential.py | 23 ++---------- tests/app/test_webauthn_server.py | 35 ++++++++++--------- tests/conftest.py | 16 ++++++++- 4 files changed, 40 insertions(+), 49 deletions(-) diff --git a/tests/app/main/views/test_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py index bf96e6d45..769725634 100644 --- a/tests/app/main/views/test_webauthn_credentials.py +++ b/tests/app/main/views/test_webauthn_credentials.py @@ -2,7 +2,6 @@ import pytest from fido2 import cbor from flask import url_for -from app import webauthn_server from app.models.webauthn_credential import RegistrationError @@ -21,18 +20,10 @@ def test_begin_register_returns_encoded_options( mocker, platform_admin_user, platform_admin_client, + webauthn_dev_server, ): - # 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_) mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[]) - - response = platform_admin_client.get( - url_for('main.webauthn_begin_register') - ) + response = platform_admin_client.get(url_for('main.webauthn_begin_register')) assert response.status_code == 200 @@ -50,7 +41,7 @@ def test_begin_register_returns_encoded_options( relying_party_options = webauthn_options['rp'] assert relying_party_options['name'] == 'GOV.UK Notify' - assert relying_party_options['id'] == 'localhost' + assert relying_party_options['id'] == 'webauthn.io' def test_begin_register_includes_existing_credentials( diff --git a/tests/app/models/test_webauthn_credential.py b/tests/app/models/test_webauthn_credential.py index 44294b8d4..07a7a2006 100644 --- a/tests/app/models/test_webauthn_credential.py +++ b/tests/app/models/test_webauthn_credential.py @@ -4,7 +4,6 @@ import pytest from fido2 import cbor from fido2.cose import ES256 -from app import webauthn_server from app.models.webauthn_credential import RegistrationError, WebAuthnCredential # noqa adapted from https://github.com/duo-labs/py_webauthn/blob/90e3d97e0182899a35a70fc510280b4082cce19b/tests/test_webauthn.py#L14-L24 @@ -18,20 +17,7 @@ ATTESTATION_OBJECT = base64.b64decode(b'o2NmbXRoZmlkby11MmZnYXR0U3RtdKJjc2lnWEgw UNSUPPORTED_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+ALfYtpAECAyUhWCAxnqAfESXOYjKUc2WACuXZ3ch0JHxV0VFrrTyjyjIHXCJYIFnx8H87L4bApR4M+hPcV+fHehEOeW+KCyd0H+WGY8s6') # noqa -@pytest.fixture -def disable_webauthn_origin_verification(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_) - - -def test_from_registration_verifies_response(disable_webauthn_origin_verification): +def test_from_registration_verifies_response(webauthn_dev_server): registration_response = { 'clientDataJSON': CLIENT_DATA_JSON, 'attestationObject': ATTESTATION_OBJECT, @@ -47,7 +33,7 @@ def test_from_registration_verifies_response(disable_webauthn_origin_verificatio assert credential_data.public_key[3] == ES256.ALGORITHM -def test_from_registration_encodes_as_unicode(disable_webauthn_origin_verification): +def test_from_registration_encodes_as_unicode(webauthn_dev_server): registration_response = { 'clientDataJSON': CLIENT_DATA_JSON, 'attestationObject': ATTESTATION_OBJECT, @@ -62,9 +48,6 @@ def test_from_registration_encodes_as_unicode(disable_webauthn_origin_verificati def test_from_registration_handles_library_errors(app_): - # enable origin verification for non-HTTPS test - webauthn_server.init_app(app_) - registration_response = { 'clientDataJSON': CLIENT_DATA_JSON, 'attestationObject': ATTESTATION_OBJECT, @@ -76,7 +59,7 @@ def test_from_registration_handles_library_errors(app_): assert 'Invalid origin' in str(exc_info.value) -def test_from_registration_handles_unsupported_keys(disable_webauthn_origin_verification): +def test_from_registration_handles_unsupported_keys(webauthn_dev_server): registration_response = { 'clientDataJSON': CLIENT_DATA_JSON, 'attestationObject': UNSUPPORTED_ATTESTATION_OBJECT, diff --git a/tests/app/test_webauthn_server.py b/tests/app/test_webauthn_server.py index 23093378b..c2bc70381 100644 --- a/tests/app/test_webauthn_server.py +++ b/tests/app/test_webauthn_server.py @@ -3,33 +3,36 @@ import pytest from app import webauthn_server +@pytest.fixture +def app_with_mock_config(mocker): + app = mocker.Mock() + + app.config = { + 'ADMIN_BASE_URL': 'https://www.notify.works', + 'NOTIFY_ENVIRONMENT': 'development' + } + + return app + + @pytest.mark.parametrize(('environment, allowed'), [ ('development', True), ('production', False) ]) def test_server_origin_verification( - app_, - mocker, + app_with_mock_config, environment, allowed ): - mocker.patch.dict( - app_.config, - values={'NOTIFY_ENVIRONMENT': environment} - ) - webauthn_server.init_app(app_) - assert app_.webauthn_server._verify('fake-domain') == allowed + app_with_mock_config.config['NOTIFY_ENVIRONMENT'] = environment + webauthn_server.init_app(app_with_mock_config) + assert app_with_mock_config.webauthn_server._verify('fake-domain') == allowed def test_server_relying_party_id( - app_, + app_with_mock_config, 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' + webauthn_server.init_app(app_with_mock_config) + assert app_with_mock_config.webauthn_server.rp.id == 'www.notify.works' diff --git a/tests/conftest.py b/tests/conftest.py index 36950f562..c9fe05cac 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,7 +13,7 @@ from flask import Flask, url_for from notifications_python_client.errors import HTTPError from notifications_utils.url_safe_token import generate_token -from app import create_app +from app import create_app, webauthn_server from . import ( TestClient, @@ -3245,6 +3245,20 @@ def set_config_values(app, dict): app.config[key] = old_values[key] +@pytest.fixture +def webauthn_dev_server(app_, mocker): + overrides = { + 'NOTIFY_ENVIRONMENT': 'development', + 'ADMIN_BASE_URL': 'https://webauthn.io', + } + + with set_config_values(app_, overrides): + webauthn_server.init_app(app_) + yield + + webauthn_server.init_app(app_) + + @pytest.fixture(scope='function') def valid_token(app_, fake_uuid): return generate_token(