Commit Graph

11457 Commits

Author SHA1 Message Date
David McDonald
9d2f495df8 Merge pull request #3900 from alphagov/make-help
Add help text for new make steps
2021-05-25 11:31:39 +01:00
David McDonald
f4dc9e15e1 Add help text for new make steps
Helpful for when you run `make` as you've forgotten what the command is
and you don't want to go find the readme
2021-05-25 11:21:39 +01:00
Chris Hill-Scott
2d2c82ca87 Merge pull request #3885 from alphagov/live-broadcast-tour
Add a version of the tour for live services
2021-05-24 10:58:45 +01:00
Andrew White
438b7457cd Merge pull request #3895 from alphagov/disable-statistics-check
Disable the remaining messages check for uploads
2021-05-22 07:34:08 +01:00
Andrew White
00c3943222 Disable the remaining messages check for uploads
The HTTP request for the statistics is taking more 30 seconds which leads to 504 errors from CloudFront.
2021-05-22 07:25:07 +01:00
Ben Thorner
658e40b6c5 Merge pull request #3883 from alphagov/rename-app_-2
Rename "app_" fixture to "notify_admin"
2021-05-19 11:53:09 +01:00
Chris Hill-Scott
5de1c4f6ca Redirect newly-created users to broadcast tour 2021-05-19 11:48:59 +01:00
Chris Hill-Scott
7697cdb2b3 Combine tests using parametrize 2021-05-19 11:48:59 +01:00
Chris Hill-Scott
016f38db9b Refactor service name code to reduce duplication
This makes the code shareable between:
- the broadcast tour pages
- the broadcast settings platform admin page
- the regular service navigation

On the training mode tour pages we don’t want to confuse people with the
organisation name or _Switch service_ links, so those are omitted and
the code is therefore slightly different.
2021-05-19 11:48:59 +01:00
Ben Thorner
5bfce61bcf Rename "app_" fixture to "notify_admin"
This naming was introduced in 2016 without explanation [1]. I find it
confusing because:

- It's reminiscent of "_app", which is a Python convention indicating
the variable is internal, so maybe avoid using it.

- It suggests there's some other "app" fixture I should be using (there
isn't, though).

The Python style guide describes using an underscore suffix to avoid
clashes with inbuilt names [1], which is sort of applicable if we need
to import the "app" module [2]. However, we can also avoid clashes by
choosing a different name, without the strange underscore.

[1]: 3b1d521c10
[2]: 78824f54fd/tests/app/main/views/test_forgot_password.py (L5)
2021-05-19 11:44:20 +01:00
Ben Thorner
03295eb828 Merge pull request #3889 from alphagov/webauthn-errors
Handle errors when registration fails
2021-05-19 11:22:05 +01:00
Chris Hill-Scott
766df5d1ca Add a version of the tour for live services
At the moment if you’re invited to a live broadcast service you get the
training mode tour. This is misleading, and could make people think they
weren’t in danger of sending a real alert.

This commit adds a short, 2 step tour for users invited to a live
broadcast service.
2021-05-19 09:41:58 +01:00
Chris Hill-Scott
af4bdf4620 Merge pull request #3881 from alphagov/confirm-live-broadcast
Add a confirmation checkbox for live broadcasts
2021-05-19 09:19:45 +01:00
Chris Hill-Scott
f10e3661b9 Remove duplicative test
Duplicated the assertions in `test_request_approval`
2021-05-18 15:58:41 +01:00
Chris Hill-Scott
ef79edba09 Remove uneccessary parametrize
It proves the behaviour isn't dependent on the channel, but there are
other variables we could equally prove that for, which we're not testing
here.
2021-05-18 15:58:41 +01:00
Chris Hill-Scott
8db280317f Rename helper method
This is ambiguous, as it's not used in a lot of the views.

There’s precendent for naming these kind of methods here:
06cc5b58c7/app/main/views/broadcast.py (L238)
2021-05-18 15:58:41 +01:00
Chris Hill-Scott
cee2a4cb7f Remove check for ARIA role
This is testing something built into GOV.UK Frontend, we don’t need to
test it ourselves.
2021-05-18 15:58:41 +01:00
Chris Hill-Scott
859674db38 Remove duplicative test
This case was already covered by `test_confirm_approve_non_training_broadcasts_errors_if_not_ticked`
2021-05-18 15:58:41 +01:00
Chris Hill-Scott
e2ef8cd36e Show error message if checkbox wasn’t checked
Because we were redirecting in all cases the error message wasn’t being
shown.

This commit changes the endpoint to respond with content (including an
error message) if the `POST` is not successful.
2021-05-18 15:58:41 +01:00
Chris Hill-Scott
7d66dadcd7 Add a confirmation checkbox for live broadcasts
We want people to be really sure before sending a live broadcast, not
just clicking through the green buttons.

This commit adds a checkbox which explains exactly the consequences of
what they’re about to do, tailored to the channel they’re on, and the
area chosen by the person creating the alert.
2021-05-18 15:58:41 +01:00
Ben Thorner
773a51dec1 Merge pull request #3891 from alphagov/try-fix-webauthn-js
Only check for WebAuthn where it's used
2021-05-18 13:36:50 +01:00
Ben Thorner
4575ad0fcd Only check for WebAuthn where it's used
This scopes the check for WebAuthn API to the page where we need
it, which will slightly reduce load times for other pages. Since
we want this script to execute ASAP, I've added a new block for
extra JS to run at the start of the body.
2021-05-18 13:01:45 +01:00
Ben Thorner
c09f0f0e06 Merge pull request #3886 from alphagov/protect-no-webauthn-js
Prevent registration if WebAuthn is not supported
2021-05-18 11:59:00 +01:00
Ben Thorner
2039d3aa45 Prevent registration if WebAuthn is not supported
This hides the "Register" button and shows an error that's specific
to one of two ways a browser may not support WebAuthn:

- JavaScript is disabled (there's no possible fallback for this).
- WebAuthn API is not supported (e.g. on Internet Explorer).

We could add a similar check for the API in the JS code to handle
the button click, but hiding it seems like enough protection.

In order to avoid elements flashing when the page loads, this uses
a view macro to embed a script at the start of the body element,
which is the same approach used for the "js-enabled" class flag [1].

Tested with Chrome and IE 11.

[1]: https://github.com/alphagov/govuk-frontend/blob/main/src/govuk/template.njk#L31
2021-05-18 10:28:25 +01:00
Ben Thorner
fd6329b92e 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.
2021-05-17 12:18:28 +01:00
Ben Thorner
8502827afb 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]: 142587b3e6/fido2/server.py (L255)
[2]: c42d9628a4/fido2/attestation/base.py (L39)
[3]: c42d9628a4/fido2/cose.py (L92)
2021-05-17 12:18:24 +01:00
Ben Thorner
bcf2f7dccd Fix errorType parameter being null
The previous syntax expected the argument would be passed as an
object like { errorType: <type> }.
2021-05-14 17:31:43 +01:00
Sakis
06cc5b58c7 Merge pull request #3888 from alphagov/add-splunk-service
Add splunk service
2021-05-14 19:20:31 +03:00
Katie Smith
3e8b872c32 Merge pull request #3887 from alphagov/link-to-api-webauthn
Integrate with the API for adding and getting webauthn creds
2021-05-14 14:47:13 +01:00
Katie Smith
bafcc02b7d Integrate with the API for adding and getting webauthn creds
This links up the `get_webauthn_credentials_for_user` and
`create_webauthn_credential_for_user` methods of the user api client to
notifications-api.

To send data to the API we need strings to be unicode, so we call
decode('utf-8') on base64 objects.

Co-authored-by: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
2021-05-14 14:28:24 +01:00
sakisv
499c8765ed Add splunk service
This will allow shipping app and router logs to splunk[1]

1: https://github.com/alphagov/paas-csls-splunk-broker/blob/main/docs/user-guide.md
2021-05-14 11:10:16 +03:00
Chris Hill-Scott
362189d562 Merge pull request #3879 from alphagov/add-government-channel
Add an option to set a service to the government channel for emergency alerts
2021-05-13 15:10:15 +01:00
Pea Tyczynska
714afff156 Merge pull request #3884 from alphagov/add-webauthn-as-auth-type
Add webauthn as an auth type
2021-05-13 14:32:03 +01:00
Ben Thorner
a7d7cb3421 Merge pull request #3878 from alphagov/register-security-key
Allow registering WebAuthn authenticators in memory
2021-05-13 12:43:16 +01:00
Ben Thorner
f4ab8776ef Fix confusing error messages when debugging
Previously a bug in the first test would lead to a 'not implemented'
console error, which isn't the actual problem. This ensures alert()
is just a simple no-op, so we can concentrate on actual errors.
2021-05-13 10:22:29 +01:00
Ben Thorner
9c983b8941 Restore all mocked values after tests
In response to [1].

This prevents mocked values leaking between tests [2]. I did try to
set 'mockRestore: true' in jest.config, but that means the restore
happens before _every_ test, which isn't what we want.

[1]: https://github.com/alphagov/notifications-admin/pull/3878#discussion_r631060116
[2]: https://jestjs.io/docs/jest-object#jestrestoreallmocks
2021-05-13 10:22:28 +01:00
Ben Thorner
35507683ee Switch to jest.spyOn() for window.location
The previous comment was incorrect, so there's no need to use the
defineObject hack, or to populate the object beforehand.
2021-05-13 10:22:27 +01:00
Ben Thorner
aae01bf8e2 Switch to jest.spyOn for navigator.credentials
This follows the same approach as for window.fetch, using the Jest
before/afterAll() blocks to handle the idiosynchrosies of whether
the object/function is defined in the test environment.
2021-05-13 10:22:26 +01:00
Ben Thorner
9ee01a2567 Check for response.ok in fetch calls
It's possible for a call to fetch to trigger then "then" callback
even thought the response is an error [1]. We should test for both
scenarios, since they are handled differently. To avoid duplicating
the tests, I've used Jest's parameterisation feature [2].

[1]: https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
[2]: https://jestjs.io/docs/api#testeachtablename-fn-timeout
2021-05-13 10:22:26 +01:00
Ben Thorner
6948f5f003 Switch to window.fetch for AJAX calls
In response to [1]. Using window.fetch means we don't get console
logs on errors, so this simplifies the error handling, although we
need to account for some errors not being a standard error object,
such as the string we get by doing Promise.reject('error').

In making this change, I've also started addressing another comment
in the PR [2], so that we reset mocked objects after the tests.

This also switches the ordering of done(), so that it's the last
statement (in response to [3]).

In the next commit we'll check for 'response.ok', but I wanted to
keep this one simple, as it's quite a large change.

[1]: https://github.com/alphagov/notifications-admin/pull/3878#discussion_r631054187
[2]: https://github.com/alphagov/notifications-admin/pull/3878#discussion_r631060116
[3]: https://github.com/alphagov/notifications-admin/pull/3878#discussion_r631061628
2021-05-13 10:22:25 +01:00
Ben Thorner
957dba4356 Avoid registering the same authenticator twice
This passes existing credentials in the server response, to allow
the browser to prevent re-registering the same key for the same
user. Registering the same key multiple times doesn't seem to be
an issue technically; the user has likely got their keys mixed up.

- Chrome says "you don't need to register it again".
- Safari exits with an InvalidStateError.
- Firefox exits with a DOMException.
2021-05-13 10:22:24 +01:00
Ben Thorner
e2cf3e2c70 Support registering a new authenticator
This adds Yubico's FIDO2 library and two APIs for working with the
"navigator.credentials.create()" function in JavaScript. The GET
API uses the library to generate options for the "create()" function,
and the POST API decodes and verifies the resulting credential. While
the options and response are dict-like, CBOR is necessary to encode
some of the byte-level values, which can't be represented in JSON.

Much of the code here is based on the Yubico library example [1][2].

Implementation notes:

- There are definitely better ways to alert the user about failure, but
window.alert() will do for the time being. Using location.reload() is
also a bit jarring if the page scrolls, but not a major issue.

- Ideally we would use window.fetch() to do AJAX calls, but we don't
have a polyfill for this, and we use $.ajax() elsewhere [3]. We need
to do a few weird tricks [6] to stop jQuery trashing the data.

- The FIDO2 server doesn't serve web requests; it's just a "server" in
the sense of WebAuthn terminology. It lives in its own module, since it
needs to be initialised with the app / config.

- $.ajax returns a promise-like object. Although we've used ".fail()"
elsewhere [3], I couldn't find a stub object that supports it, so I've
gone for ".catch()", and used a Promise stub object in tests.

- WebAuthn only works over HTTPS, but there's an exception for "localhost"
[4].  However, the library is a bit too strict [5], so we have to disable
origin verification to avoid needing HTTPS for dev work.

[1]: c42d9628a4/examples/server/server.py
[2]: c42d9628a4/examples/server/static/register.html
[3]: 91453d3639/app/assets/javascripts/updateContent.js (L33)
[4]: https://stackoverflow.com/questions/55971593/navigator-credentials-is-null-on-local-server
[5]: c42d9628a4/fido2/rpid.py (L69)
[6]: https://stackoverflow.com/questions/12394622/does-jquery-ajax-or-load-allow-for-responsetype-arraybuffer
2021-05-13 10:22:23 +01:00
Pea Tyczynska
2a756f90d4 Add webauth as an auth type
When showing what auth type user uses to sign in, add a text
for users with webauthn.

On password change or sign in, throw not implemented error
if user uses webauthn auth.
2021-05-12 17:40:31 +01:00
Chris Hill-Scott
ad0b7537de Make the government channel visually distinct
It’s really serious, so this sets it apart from the other live channels.
2021-05-12 16:22:45 +01:00
Chris Hill-Scott
d38f44ec69 Be explicit that ‘test’ goes out on real networks
Adding ‘all networks’ whenever we mention the using the test channel
without a restriction to a single network should help reinforce that
this sends real alerts.
2021-05-12 16:22:45 +01:00
Chris Hill-Scott
e45bd485e8 Add conditional reveal to network selection
Rather than try to explain all/only just through words we can use some
interaction design to make the hierarchy of choices more explicit.
2021-05-12 16:22:44 +01:00
Chris Hill-Scott
d720b0e47a Rename cell broadcasts
‘Emergency alerts’ is the confirmed name of the service now.
2021-05-12 16:22:44 +01:00
Chris Hill-Scott
a0f54539cc Add a second step for choosing networks
Only the test channel has the option to isolate messages to one network.

This commits makes the choices less confusing by only showing the
network choice to those who have selected the test channel.
2021-05-12 16:22:44 +01:00
Chris Hill-Scott
f640767f3d Add government channel
We have been asked to support the government channel so that:
- it can be tested
- the option to use it is available for the most severe of emergencies,
  where the public’s choice to opt-out is outweighed by the widespread
  risk to life
2021-05-12 16:22:43 +01:00
Chris Hill-Scott
ffd844b2a7 Add confirmation step to emergency alert settings
It feels quite dangerous that it’s just one click to make an emergency
alerts service live.

This commit adds a confirmation step which explains the consequences of
what you’re about to do.
2021-05-12 14:53:49 +01:00