We don't use this term consistently and it's not defined anywhere.
Since most of the Admin app deals with user-facing permssions, it's
OK to just use the term "permissions". Where both types of permission
are present in the same file, we can more clearly distinguish them
as "UI permissions" and "DB permissions".
These are more than a list of permissions: each item includes the
label to use when displaying it as an option on a form. Switching
to a name that reflects how the attributes are used will help to
avoid confusion when we rename some of the other attributes in the
same file in later commits.
This is so we can allow the sender name 'UC' for DWP.
Note, this is specifically only straight single quotes and not curly
quotes or double quotes. Curly quotes are not supported in the GSM
character set (https://en.wikipedia.org/wiki/GSM_03.38). There is
currently no defined user ask to support double quotes in sms sender
names.
I have tested this by sending a message through both Firetext and MMG to
make sure they both support the single quote character in SMS sender
names.
DWP also have had no particular issues using the SMS sender name with
their existing system in the past either.
The `send_messages` permission has been deprecated for use with
broadcast services, so we can drop support for it in the code. We
were supporting both the old permissions and new permissions
(`create_broadcasts` and `approve_broadcasts`) while we switched people
over.
This removes `send_messages` from the `user_has_permissions` decorator
around the broadcast routes and from the page to view a broadcast and
broadcast dashboards. We can now git rid of a lot of the parameterization
that was temporarily added to the tests.
At the moment we say that you either ‘add’ an alert or ‘send’ it.
This is confusing because:
- an alert isn’t received on people’s phones until it’s approved, so
this is really when it is ‘sent’ conceptually
- an alert can be rejected before anyone receives it, so the UI can say
an alert that no-one ever received was sent
This commit re-labels things so that the the first part of the process
is ‘creating’ the alert.
This makes all the permissions nice and distinct from each other. Adding
templates and adding alerts feel conceptually quite different things
(what are you adding the alert to?).
previously we'd skip the template page entirely if someone didnt have
manage templates/api keys permission. however, if the template is
deleted you'd then go through the flow entering placeholders and stuff
before it would then crash when trying to send.
instead, just bounce the user to the template page. It has the content
and says when the template was deleted.
I've used the term "admin_roles" in the event data to try and show
that these are not the permissions we store in the DB. This is the
name we use for the abstracted form of permissions in the Admin app.
While we could store the DB permissions, that would be a bit more
effort and arguably it's clearer to keep the event data consistent
with the options the user actually saw / chose.
Added two new permissions - `create_broadcasts` and
`approve_broadcasts`. These new permissions get added to the
`has_permissions` decorator of the broadcast routes to allow the routes
to be accessed with either the old permissions on the new ones while we
switch over.
We were using the `send_messages` permission for the broadcast routes.
By having two new permissions we can allow a more granular control of
these routes.
It’s exactly the same code as `BaseInviteUserForm` so there’s really no
need to duplicate it (and means that changes we make to
`BaseInviteUserForm` in the future will get inherited).
The reason the email address is considered invalid is because it is the
address of the person doing the inviting.
This commit renames the variable to be more specific and avoid confusion
with the email address of the person being invited.
We don’t want a single person to have two accounts on an emergency
alerts service because it would let them circumvent the two eyes
approval process.
We can go some way to mitigating against this by stopping people using
common methods that email providers use to alias email addresses. These
are:
- being case insensitive
- being insensitive to the position or number of dots in the local part
of an email address
- using ‘plus addressing’
We already prevent the first one, this commit adds normalisation which
strip out the second two before doing the comparision with the current
user’s email address.
Previously the backend would never validate permissions because the
"not service.active" part would (usually) fail. I've updated it to
match the (inverse of the) conditional we have in the HTML [1].
[1]: 6ac593aa5f/app/templates/views/service-settings.html (L455)
When the broadcast service settings form is submitted it now removes all
permissions for users in notifications-api. This means it should be
clearing the user cache.
This means that we can rewrite `validate_email_address` to do a
different comparison without having to also change `__init__`
I’ve moved the platform admin check into its own conditional to keep the
line length manageable.
For most events this makes the purpose of each argument clearer at
the point the event is called. It's still worth having a function
for each event type, as this abstracts knowledge of the event label.
Using a schema approach will make adding new events easier.
In the next commit we'll DRY-up the duplication in the tests as well.
This is particularly important for broadcast services, where a rogue
service or platform admin could launch a DoS attack by suspending a
service at a critical moment when it needs to send alerts.
This allows us to roll out the feature to other users. Note that
the flag is also "True" if the user has "webauthn_auth" as their
auth type, so this is compatible with the more fine-grained check
we have on the authentication parts of the feature. We could do a
more explicit "can_use_webauthn or webauthn_auth" check here, but
the idea is that we'll be able to get rid of this flag eventually,
so I've optimised for brevity instead.
I've modified a couple of the unhappy-path tests to make it more
explicit that the flag is false, since it can be true for Platform
Admins and "normal users" alike.
Previously we applied this restriction to Platform Admins, on the
assumption that all of them use a security key to log in. Rather
than making that assumption, we can explicitly check their login
method, which also supports rolling out the feature to more users.
We put some content in the go live ticket which is for our benefit, for
example notes about the organisation.
It’s hard for us to be able to say what we want here if we know that the
person making a go live request is going to see those notes.
This commit changes go live requests so that the initial content of the
ticket is hidden from the person raising it (in Zendesk it will appear
as an ‘internal note’, rather than a ‘public reply’).
---
Depends on:
- [ ] https://github.com/alphagov/notifications-utils/pull/877/files
We had some kind of idea that having this empty page would introduce the
idea of choosing areas and reinforce that you are building up a list of
areas.
But since the journey is now so simple with the button to create an
alert directly on the dashboard page, maybe people don’t need this extra
orientation.
Since the register and authentication APIs work in pairs, we can
just put the restrictions on the "begin" API. We weren't testing
the restrictions on the "complete" API anyway.
For authentication, it's also enough to check if the user has
WebAuthn as their auth type, as it's not a big deal if a user
continues to login with a security key indefinitely.
It should be enough to check the user has it set as their auth type.
Even if a user is no longer eligible to register a security key, it
should still be OK for them to continue using the feature.
We signal that we're mid-way through the sign-in flow by adding a
`user_details` dict to the session.
previously, we'd only put a user's details in the session in `User.sign_in`,
just before sending any 2fa prompt and redirecting to the two factor
pages.
However, we found a bug where a user with no session (eg, using a fresh
browser) tried to log in, but they had never clicked the link to
validate their email address when registering. Their user's state was
still in "pending", so we redirected to `main.resend_email_verification`
as intended - however, they didn't have anything in the session and the
resend page expected to get the email address to resend to out of that.
To be safe, as soon as we've confirmed the user has entered their
password correctly, lets save the session data at that point. That way
any redirects will be fine.
We hide the radio field in the HTML for platform admins, as we don't
want anyone to be able to change their auth type. However, when the form
is validated, the form has a field called login_authentication that it
expects a value for. It silently fails as it complains that when the
user POSTed they didn't select a value for that radio field, but the
error message is on the radio fields that don't get displayed to the
user so they'd never know.
Fixing this is actually pretty hard.
We use this form in two places, one where we have a user to edit, one
where we are creating an invite from scratch. So sometimes we don't know
about a user's auth type. In addition, radio buttons are mandatory by
design, but now sometimes we don't just want to make it optional but
explicitly ignore the value being passed in? To solve this, remove the
field entirely from the form if the user is a platform admin. This means
that if the code in manage_users.py tries to access the
login_authentication value from the form, it'll error, but I think
that's okay to leave for now given we concede that this isn't a perfect
final solution.
The tests didn't flag this previously as they tried to set from sms_auth
(the default for `platform_admin_user`) TO email_auth or sms_auth. Also,
the diagnosis of this bug was confounded further by the fact that
`mock_get_users_by_service` sets what is returned by the API - the
service model then takes the IDs out of that response and calls
`User.get_user_by_id` for the matching ID (as in, the code only uses
get_users_by_service to ensure the user belongs to that service). This
means that we accidentally set the form editing the current user, as
when we log in we set `get_user_by_id` to return the user of our choice
