pass nextUrl through yubikey flow

the next url comes from sign in via a query param, and needs to go to
the POST /webauthn/authenticate endpoint. That endpoint logs the user
in and returns the redirect to the browser, and will take the next from
the request query params to get there.

also moving the window mocks to beforeEach/afterEach ensures that
promise callbacks from previous tests aren't still associated in future
tests to ensure good test isolation.

unfortunately i couldn't get mocking location for a single js test to
work, but by changing the global config i was able to add some query
params that i can expect to be passed through. Don't love this at all
but not quite sure of a good way round this. I think we're not
practicing very good hygiene and best practices with our mocking and
it's really confounding me here.
This commit is contained in:
Leo Hemsted
2021-05-27 12:07:11 +01:00
parent 5ea82b0cdc
commit bb7343d846
4 changed files with 95 additions and 14 deletions

View File

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

View File

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

View File

@@ -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 type="submit" data-module="authenticate-security-key" data-csrf-token="abc123"></button>
`
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'],

View File

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