darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-07-10 03:14:58 -04:00
Code Issues Packages Projects Releases Wiki Activity

show error message in banner rather than an alert

Browse Source 
the banner is a nicer user experience, and consistent with how we
display errors elsewhere in notify. For now pass through the error
message from JS, but we'll probably want to change that since the erorr
messages themselves are often a bit cryptic and unhelpful
This commit is contained in:
Leo Hemsted
2021-08-11 18:02:50 +01:00
parent c96a1dc0b7
commit 0b27d7e0a9
6 changed files with 64 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
app/assets/javascripts/authenticateSecurityKey.js
View File

@@ -7,6 +7,9 @@
.on('click', function (event) {
event.preventDefault();
// hide any existing error prompt
window.GOVUK.ErrorBanner.hideBanner();
fetch('/webauthn/authenticate')
.then(response => {
if (!response.ok) {
@@ -67,9 +70,8 @@
.catch(error => {
console.error(error);
// some browsers will show an error dialogue for some
// errors; to be safe we always pop up an alert
var message = error.message || error;
alert('Error during authentication.\n\n' + message);
// errors; to be safe we always display an error message on the page.
window.GOVUK.ErrorBanner.showBanner('Something went wrong');
});
});
};

9
app/assets/javascripts/registerSecurityKey.js
View File

@@ -7,6 +7,9 @@
.on('click', function(event) {
event.preventDefault();
// hide any existing error prompt
window.GOVUK.ErrorBanner.hideBanner();
fetch('/webauthn/register')
.then((response) => {
if (!response.ok) {
@@ -44,9 +47,9 @@
.catch((error) => {
console.error(error);
// some browsers will show an error dialogue for some
// errors; to be safe we always pop up an alert
var message = error.message || error;
alert('Error during registration.\n\n' + message);
// errors; to be safe we always display an error message on the page.
const message = error.message || error;
window.GOVUK.ErrorBanner.showBanner('Something went wrong');
});
});
};

9
app/templates/views/two-factor-webauthn.html
View File

@@ -21,6 +21,15 @@
{% block maincolumn_content %}
{{ govukErrorMessage({
"classes": "banner-dangerous govuk-!-display-none",
"text": "There was a javascript error.",
"attributes": {
"aria-live": "polite",
"tabindex": '-1'
}
}) }}
<div class="govuk-grid-row">
<div class="govuk-grid-column-one-half">
{{ page_header(page_title) }}

10
app/templates/views/user-profile/security-keys.html
View File

@@ -46,6 +46,16 @@
{% endset %}
<div class="govuk-grid-row">
{{ govukErrorMessage({
"classes": "banner-dangerous govuk-!-display-none",
"text": "There was a javascript error.",
"attributes": {
"aria-live": "polite",
"tabindex": '-1'
}
}) }}
{% if credentials %}
<div class="govuk-grid-column-five-sixths">
{{ page_header(page_title) }}

34
tests/javascripts/authenticateSecurityKey.test.js
View File

@@ -5,6 +5,10 @@ beforeAll(() => {
// populate missing values to allow consistent jest.spyOn()
window.fetch = () => { }
window.navigator.credentials = { get: () => { } }
window.GOVUK.ErrorBanner = {
showBanner: () => {},
hideBanner: () => {}
};
})
afterAll(() => {
@@ -23,9 +27,6 @@ describe('Authenticate with security key', () => {
// you might need to comment this out to debug some failures
jest.spyOn(console, 'error').mockImplementation(() => { })
// ensure window.alert() is implemented to simplify errors
jest.spyOn(window, 'alert').mockImplementation(() => { })
document.body.innerHTML = `
<button type="submit" data-module="authenticate-security-key" data-csrf-token="abc123"></button>`
@@ -89,8 +90,8 @@ describe('Authenticate with security key', () => {
done()
})
// this will make the test fail if the alert is called
jest.spyOn(window, 'alert').mockImplementation((msg) => {
// this will make the test fail if the error banner is displayed
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
done(msg)
})
@@ -142,7 +143,7 @@ describe('Authenticate with security key', () => {
test.each([
['network'],
['server'],
])('alerts if fetching WebAuthn fails (%s error)', (errorType, done) => {
])('errors if fetching WebAuthn fails (%s error)', (errorType, done) => {
jest.spyOn(window, 'fetch').mockImplementation((_url) => {
if (errorType == 'network') {
return Promise.reject('error')
@@ -151,15 +152,15 @@ describe('Authenticate with security key', () => {
}
})
jest.spyOn(window, 'alert').mockImplementation((msg) => {
expect(msg).toEqual('Error during authentication.\n\nerror')
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
expect(msg).toEqual('Something went wrong')
done()
})
button.click()
})
test('alerts if comms with the authenticator fails', (done) => {
test('errors if comms with the authenticator fails', (done) => {
jest.spyOn(window, 'fetch')
.mockImplementationOnce((_url) => {
const webauthnOptions = window.CBOR.encode('someArbitraryOptions')
@@ -173,8 +174,8 @@ describe('Authenticate with security key', () => {
return Promise.reject(new DOMException('error'))
})
jest.spyOn(window, 'alert').mockImplementation((msg) => {
expect(msg).toEqual('Error during authentication.\n\nerror')
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
expect(msg).toEqual('Something went wrong')
done()
})
@@ -184,7 +185,7 @@ describe('Authenticate with security key', () => {
test.each([
['network error'],
['internal server error'],
])('alerts if POSTing WebAuthn credentials fails (%s)', (errorType, done) => {
])('errors if POSTing WebAuthn credentials fails (%s)', (errorType, done) => {
jest.spyOn(window, 'fetch')
.mockImplementationOnce((_url) => {
const webauthnOptions = window.CBOR.encode('someArbitraryOptions')
@@ -220,8 +221,9 @@ describe('Authenticate with security key', () => {
}
})
jest.spyOn(window, 'alert').mockImplementation((msg) => {
expect(msg).toEqual('Error during authentication.\n\nerror')
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
expect(msg).toEqual('Something went wrong')
done()
})
@@ -266,8 +268,8 @@ describe('Authenticate with security key', () => {
done()
})
// this will make the test fail if the alert is called
jest.spyOn(window, 'alert').mockImplementation((msg) => {
// this will make the test fail if the error banner is displayed
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
done(msg)
})

31
tests/javascripts/registerSecurityKey.test.js
View File

@@ -4,7 +4,11 @@ beforeAll(() => {
// populate missing values to allow consistent jest.spyOn()
window.fetch = () => {}
window.navigator.credentials = { create: () => {} }
window.navigator.credentials = { create: () => { } }
window.GOVUK.ErrorBanner = {
showBanner: () => { },
hideBanner: () => { }
};
})
afterAll(() => {
@@ -23,9 +27,6 @@ describe('Register security key', () => {
// you might need to comment this out to debug some failures
jest.spyOn(console, 'error').mockImplementation(() => {})
// ensure window.alert() is implemented to simplify errors
jest.spyOn(window, 'alert').mockImplementation(() => {})
document.body.innerHTML = `
<a href="#" role="button" draggable="false" class="govuk-button govuk-button--secondary" data-module="register-security-key">
Register a key
@@ -78,8 +79,8 @@ describe('Register security key', () => {
done()
})
// this will make the test fail if the alert is called
jest.spyOn(window, 'alert').mockImplementation((msg) => {
// this will make the test fail if the error banner is displayed
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
done(msg)
})
@@ -89,7 +90,7 @@ describe('Register security key', () => {
test.each([
['network'],
['server'],
])('alerts if fetching WebAuthn options fails (%s error)', (errorType, done) => {
])('errors if fetching WebAuthn options fails (%s error)', (errorType, done) => {
jest.spyOn(window, 'fetch').mockImplementation((_url) => {
if (errorType == 'network') {
return Promise.reject('error')
@@ -98,8 +99,8 @@ describe('Register security key', () => {
}
})
jest.spyOn(window, 'alert').mockImplementation((msg) => {
expect(msg).toEqual('Error during registration.\n\nerror')
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
expect(msg).toEqual('Something went wrong')
done()
})
@@ -110,7 +111,7 @@ describe('Register security key', () => {
['network error'],
['internal server error'],
['bad request'],
])('alerts if sending WebAuthn credentials fails (%s)', (errorType, done) => {
])('errors if sending WebAuthn credentials fails (%s)', (errorType, done) => {
jest.spyOn(window, 'fetch').mockImplementationOnce((_url) => {
// initial fetch of options from the server
@@ -140,15 +141,15 @@ describe('Register security key', () => {
}
})
jest.spyOn(window, 'alert').mockImplementation((msg) => {
expect(msg).toEqual('Error during registration.\n\nerror')
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
expect(msg).toEqual('Something went wrong')
done()
})
button.click()
})
test('alerts if comms with the authenticator fails', (done) => {
test('errors if comms with the authenticator fails', (done) => {
jest.spyOn(window.navigator.credentials, 'create').mockImplementation(() => {
return Promise.reject(new DOMException('error'))
})
@@ -162,8 +163,8 @@ describe('Register security key', () => {
})
})
jest.spyOn(window, 'alert').mockImplementation((msg) => {
expect(msg).toEqual('Error during registration.\n\nerror')
jest.spyOn(window.GOVUK.ErrorBanner, 'showBanner').mockImplementation((msg) => {
expect(msg).toEqual('Something went wrong')
done()
})