diff --git a/app/assets/javascripts/authenticateSecurityKey.js b/app/assets/javascripts/authenticateSecurityKey.js index 9a39236e3..5ce70578a 100644 --- a/app/assets/javascripts/authenticateSecurityKey.js +++ b/app/assets/javascripts/authenticateSecurityKey.js @@ -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'); }); }); }; diff --git a/app/assets/javascripts/registerSecurityKey.js b/app/assets/javascripts/registerSecurityKey.js index 2e3280e40..39005c29c 100644 --- a/app/assets/javascripts/registerSecurityKey.js +++ b/app/assets/javascripts/registerSecurityKey.js @@ -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'); }); }); }; diff --git a/app/templates/views/two-factor-webauthn.html b/app/templates/views/two-factor-webauthn.html index 0221e553e..9fd6a339a 100644 --- a/app/templates/views/two-factor-webauthn.html +++ b/app/templates/views/two-factor-webauthn.html @@ -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' + } + }) }} +
{{ page_header(page_title) }} diff --git a/app/templates/views/user-profile/security-keys.html b/app/templates/views/user-profile/security-keys.html index e1a77aa50..b20c7881b 100644 --- a/app/templates/views/user-profile/security-keys.html +++ b/app/templates/views/user-profile/security-keys.html @@ -46,6 +46,16 @@ {% endset %}
+ + {{ govukErrorMessage({ + "classes": "banner-dangerous govuk-!-display-none", + "text": "There was a javascript error.", + "attributes": { + "aria-live": "polite", + "tabindex": '-1' + } + }) }} + {% if credentials %}
{{ page_header(page_title) }} diff --git a/tests/javascripts/authenticateSecurityKey.test.js b/tests/javascripts/authenticateSecurityKey.test.js index 2b9503331..8b83af2aa 100644 --- a/tests/javascripts/authenticateSecurityKey.test.js +++ b/tests/javascripts/authenticateSecurityKey.test.js @@ -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 = ` ` @@ -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) }) diff --git a/tests/javascripts/registerSecurityKey.test.js b/tests/javascripts/registerSecurityKey.test.js index 1886935fd..03bbbd075 100644 --- a/tests/javascripts/registerSecurityKey.test.js +++ b/tests/javascripts/registerSecurityKey.test.js @@ -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 = ` 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() })