diff --git a/app/assets/javascripts/authenticateSecurityKey.js b/app/assets/javascripts/authenticateSecurityKey.js index 1326c923b..9a39236e3 100644 --- a/app/assets/javascripts/authenticateSecurityKey.js +++ b/app/assets/javascripts/authenticateSecurityKey.js @@ -21,7 +21,21 @@ return window.navigator.credentials.get(options); }) .then(credential => { - return fetch('/webauthn/authenticate', { + const currentURL = new URL(window.location.href); + + // create authenticateURL from admin hostname plus /webauthn/authenticate path + const authenticateURL = new URL('/webauthn/authenticate', window.location.href); + + const nextUrl = currentURL.searchParams.get('next'); + if (nextUrl) { + // takes nextUrl from the query string on the current browser URL + // (which should be /two-factor-webauthn) and pass it through to + // the POST. put it in a query string so it's consistent with how + // the other login flows manage it + authenticateURL.searchParams.set('next', nextUrl); + } + + return fetch(authenticateURL, { method: 'POST', headers: { 'X-CSRFToken': component.data('csrfToken') }, body: window.CBOR.encode({ diff --git a/tests/app/main/views/test_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py index 32b26dc32..011200eaa 100644 --- a/tests/app/main/views/test_webauthn_credentials.py +++ b/tests/app/main/views/test_webauthn_credentials.py @@ -332,7 +332,18 @@ def test_complete_authentication_clears_session( @freeze_time('2020-01-30') -def test_verify_webauthn_login_signs_user_in_signs_user_in(client, mocker, mock_create_event, platform_admin_user): +@pytest.mark.parametrize('url_kwargs, expected_redirect', [ + ({}, '/accounts-or-dashboard'), + ({'next': '/bar'}, '/bar'), +]) +def test_verify_webauthn_login_signs_user_in( + client, + mocker, + mock_create_event, + platform_admin_user, + url_kwargs, + expected_redirect, +): platform_admin_user['auth_type'] = 'webauthn_auth' platform_admin_user['email_access_validated_at'] = '2020-01-25T00:00:00.000000Z' @@ -345,10 +356,10 @@ def test_verify_webauthn_login_signs_user_in_signs_user_in(client, mocker, mock_ mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_authentication') mocker.patch('app.user_api_client.complete_webauthn_login_attempt', return_value=(True, None)) - resp = client.post(url_for('main.webauthn_complete_authentication')) + resp = client.post(url_for('main.webauthn_complete_authentication', **url_kwargs)) assert resp.status_code == 200 - assert cbor.decode(resp.data)['redirect_url'] == url_for('main.show_accounts_or_dashboard') + assert cbor.decode(resp.data)['redirect_url'] == expected_redirect # removes stuff from session with client.session_transaction() as session: assert 'user_details' not in session diff --git a/tests/javascripts/authenticateSecurityKey.test.js b/tests/javascripts/authenticateSecurityKey.test.js index 90bbf89f3..c2e778334 100644 --- a/tests/javascripts/authenticateSecurityKey.test.js +++ b/tests/javascripts/authenticateSecurityKey.test.js @@ -8,18 +8,10 @@ beforeAll(() => { // 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 = { get: () => { } } }) afterAll(() => { require('./support/teardown.js') - - // restore window attributes to their original undefined state - delete window.fetch - delete window.navigator.credentials }) describe('Authenticate with security key', () => { @@ -30,10 +22,23 @@ describe('Authenticate with security key', () => { ` button = document.querySelector('[data-module="authenticate-security-key"]') + + // populate missing values to allow consistent jest.spyOn() + window.fetch = () => { } + window.navigator.credentials = { get: () => { } } + window.alert = () => { } + window.GOVUK.modules.start() }) - test('authenticates a credential and redirects', (done) => { + afterEach(() => { + // restore window attributes to their original undefined state + delete window.fetch + delete window.navigator.credentials + delete window.alert + }) + + test('authenticates a credential and redirects based on the admin app response', (done) => { jest.spyOn(window, 'fetch') .mockImplementationOnce((_url) => { @@ -93,6 +98,57 @@ describe('Authenticate with security key', () => { button.click() }); + test('authenticates and passes a redirect url through to the authenticate admin endpoint', (done) => { + jest.spyOn(window, 'fetch') + .mockImplementationOnce((_url) => { + // initial fetch of options from the server + // fetch defaults to GET + // options from the server are CBOR-encoded + let webauthnOptions = window.CBOR.encode('someArbitraryOptions') + + return Promise.resolve({ + ok: true, arrayBuffer: () => webauthnOptions + }) + }) + + jest.spyOn(window.navigator.credentials, 'get').mockImplementation((options) => { + let credentialsGetResponse = { + response: { + authenticatorData: [], + signature: [], + clientDataJSON: [] + }, + rawId: [], + type: "public-key", + } + return Promise.resolve(credentialsGetResponse) + }) + + jest.spyOn(window, 'fetch') + .mockImplementationOnce((url, options = {}) => { + // subsequent POST of credential data to server + expect(url.toString()).toEqual( + 'https://www.notifications.service.gov.uk/webauthn/authenticate?next=%2Ffoo%3Fbar%3Dbaz' + ); + + // mark the test as done here as we've finished all our asserts - if something goes wrong later and + // we end up in the alert mock, that `done(msg)` will override this and mark the test as failed + done(); + + const loginResponse = window.CBOR.encode({ redirect_url: '/foo' }) + return Promise.resolve({ + ok: true, arrayBuffer: () => Promise.resolve(loginResponse) + }) + }) + + // make sure we error out if alert is called + jest.spyOn(window, 'alert').mockImplementation((msg) => { + done(msg) + }) + + button.click() + }); + test.each([ ['network'], ['server'], diff --git a/tests/javascripts/jest.config.js b/tests/javascripts/jest.config.js index adf68d50a..f686f6b69 100644 --- a/tests/javascripts/jest.config.js +++ b/tests/javascripts/jest.config.js @@ -1,4 +1,4 @@ module.exports = { setupFiles: ['./support/setup.js'], - testURL: 'https://www.notifications.service.gov.uk' + testURL: 'https://www.notifications.service.gov.uk/?next=%2Ffoo%3Fbar%3Dbaz' }