This closes a security loophole, where the auth type of a Platform
Admin could be unwittingly changed when they accept an invite, or
by an admin of a service they are a member of.
when getting a list of security keys
Also test separately that we are correctly choosing key out of list
of security keys. Previously we have done it as a part
of testing pages where where we were calling API to get a list
of keys, but then choosing one of those keys based on id.
Also remove redundant second test credential after PR review
Also remove redundant return value from mocks in update name tests
When we are unable to delete security key because it's the last
one for that user, API throws an error. Here we catch that error
and display useful message to the user.
Use security key instead of webauthn credential
in user facing message - for consistency and readability.
We use security key term in user facing stuff and webauthn
credential in the code.
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.
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.
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.
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.
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.
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
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)
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>
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.
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
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.
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.
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.
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
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.
This adds a new platform admin settings row, leading a page which
shows any existing keys and allows a new one to be registered. Until
the APIs for this are implemented, the user API client just returns
some stubbed data for manual testing.
This also includes a basic JavaScript module to do the main work of
registering a new authenticator, to be implemented in the next commits.
Some more minor notes:
- Setting the headings in the mapping_table is necessary to get the
horizontal rule along the top (to match the design).
- Setting caption to False in the mapping_table is necessary to stop
an extra margin appearing at the top.
The current_service.allowed_broadcast_provider is now always "all" or
one of the four providers, which means we can simply the code by not
checking if it is None.
Until all the data is updated to always be "all", we have to handle the
case of provider_restriction being set to None or "all" (which mean the
same thing).
The code can be tidied up once the broadcast provider_restriction is never None.
We're replacing the value of None with the value of all. API has been
updated to accept both values
(1767535def)
so this change starts sending notifications-api the value of "all".
This is the first step in making the UI easier for setting the
options for a broadcast service. Here we remove the options for
"Training mode" test channels. When we create a broadcast message for a trail mode service it is marked as stubbed and does not create a broadcast event that is sent to a provider.
The label for the form and setting page have been updated to reflect the
change.