view args are parameters within the route. for example,
`/organisation/<org_id>/users`. If there is an org_id, then check that
the user is part of that organisation (users.organisations is a list of
all orgs that user is a member of).
* platform admins ignore this check if restrict_admin_usage=False
* if an endpoint has both org_id and service_id, org_id takes
precedence, but we should probably revisit this if we ever need
to create such an endpoint.
* you now call `@user_has_permissions()` with no arguments for
organisation endpoints - we can look at this if we decide we want
more clarity.
* you should never call user_has_permissions without any arguments
for endpoints that aren't organisation-based. We'll raise
NotImplementedError if you do.
we branch on any_ to either say "require ALL these permissions" or
"require ANY of these permissions". But we only ever call the decorator
with one permission, or with any_=True, so it's unnecessary
rather than allow admins to do everything specifically, we should
only block them from things we conciously don't want them to do.
This is "Don't let platform admins send letters from services they're
not in". Everything else the platform admins can do.
This is step one, adding a restrict_admin_usage flag, and setting that
for those restricted endpoints around creating api keys, uploading CSVs
and sending one-off messages.
Also, this commit separates the two use cases for permissions:
* user.has_permission for access control
* user.has_permission_for_service for user info - this is used for
showing checkboxes on the manage-users page for example
With this, we can remove the admin_override flag from the permission
decorator.
in the db, we have several rows for single permissions - we separate
`send_messages` into `send_texts`, `send_emails` and `send_letters`,
and also `manage_service` into `manage_users` and `manage_settings`.
But on the front end we don't do anything with this distinction. It's
unhelpful for us to have to think about permissions as groups of things
when we can never split them up at all. So we should combine them. This
commit makes sure:
* when user models are read (from JSON direct from the API), we
should transform them from db permissions into roles.
* when permissions are persisted (editing permissions, and creating
invites), we should send db permissions to the API.
All other interaction with permissions (should just be the endpoint
decorator and checks in html templates generally) should use admin
roles.
when added to a service, all users are given the view_activity
permission. So, if that's included in the list, we don't need `any_`,
and we don't need any of the other permissions.
platform_admin is a separate concept to permissions, so by removing the
checks for it from the current_user.has_permissions function, we can
simplify things greatly. We already record on the user whether they're
a platform admin anyway.
If someone has duplicate recipient columns in their file we don’t know
which one to use. This commit adds an error message which should help
them fix the duplication.
This commit doesn’t go to the extra effort to actually show the
correct values for duplication in the preview. Don’t think it’s worth
the effort/complexity for how infrequently we’ve seen this error.
Depends on:
- [ ] https://github.com/alphagov/notifications-utils/pull/376
The free allowance affect the number of free text messages a services get per yer.
This allowance is being set properly when an organisation is created by not updated.
This PR updates the free allowance when the organisation type is updated.
The free allowance can also be changed if service has an exceptional free allowance.
Caught the error and displayed an error PNG so it is obvious something
failed. Currently it displayed a thumbnail of a png over the top of the
loading page, and therefore it wasn't obvious of the state.
preview document rather than go to template preview.
This will remove the logic from admin and place it in api so it is
easier to expand on later when there are precompiled PDFs
Since we send all one off messages as priority now[1], we don’t need to
explicitly mark this template as being priority.
This stops the (potential) problem of people skipping the tour, still
having this template and then modifying it to send other messages,
potentially in high volumes from CSV files or the API. I don’t think
this is a real problem now, but worth cleaning this up.
Currently:
- 827 priority templates in the database
- 195 of which are not deleted
- 18 of which are not called ‘Example text message template’
- 3 of which look like genuine use cases, not from services that we run
[1]: https://github.com/alphagov/notifications-api/pull/1722
Excel stores dates as floating point numbers, counting the days (or
fraction thereof) since 1900 (except when it counts from 1904).
However it also, incorrectly, recognises 1900 as a leap year. This means
that it’s ambiguous whether day 59 is February 28th, or February 27th,
depending if you’re counting up or down. In fact any number less than 60
is, therefore, ambiguous.
This shouldn’t really matter since no-one is going to be using dates in
the year 1900 in Notify messages. _Except_ when Excel misidentifies a
cell as containing a date. For example, if you have the number `9`
inside a cell, it means _house number 9_ if the next cell contains
_example_ street. but Excel is all like ‘oh they must want January 9th
1900!’ No. Bad Excel.
There’s not much we can do about this, but we can at least give people
an error message with the workaround, which is what this commit does.
Most of this commit message is paraphrased from:
http://xlrd.readthedocs.io/en/latest/dates.html
Having SMS senders that start with 00 can cause issues with Firetext due
to Firetext's validation rules, so we shouldn't allow SMS senders to start
with 00.
Firetext treats a double 00 at the start of the senderID as an international
prefix, so removes them. A sender of 00447876574016 would become 447876574016.
Under Firetext's validation rules, an SMS sender of five 0s (00000) would
become 4400. This is because the first 00 are removed (as the international
prefix). The third 0 is seen as the start of a phone number, and becomes 44,
leaving the final 00 = 4400.
Currently requests to the API made from the admin app are going from
PaaS admin app to the nginx router ELB, which then routes them back
to the api app on PaaS.
This makes sense for external requests, but for requests made from
the admin app we could skip nginx and go directly to the api PaaS
host, which should reduce load on the nginx instances and
potentially reduce latency of the api requests.
API apps on PaaS are checking the X-Custom-Forwarder header (which
is set by nginx on proxy_pass requests) to only allow requests going
through the proxy.
This adds the custom header to the API client requests, so that they
can pass that header check without going through nginx.
When downloading a report of a which messages from a job have been
delivered and which have failed we currently only include the Notify
data. This makes it hard to reconcile or do analysis on these reports,
because often the thing that people want to reconcile on is in the data
they’ve uploaded (eg a reference number).
Here’s an example of a user talking about this problem:
> It would also be helpful if the format of the delivery and failure
> reports could include the fields from the recipient's file. While I
> can, of course, cross-reference one report with the other it would be
> easier if I did not have to. We send emails to individuals within
> organisations and it is not always easy to establish the organisation
> from a recipient's email address. This is particularly important when
> emails fail to be delivered as we need to contact the organisation to
> establish a new contact.
– ticket 677
We’ve also seen it when doing research with a local council.
This commit takes the original file, the data from the API, and munges
them together.
Done using isort[1], with the following command:
```
isort -rc ./app ./tests
```
Adds linting to the `run_tests.sh` script to stop badly-sorted imports
getting re-introduced.
Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because
I think it gives the cleanest diffs, eg:
```
from third_party import (
lib1,
lib2,
lib3,
lib4,
)
```
1. https://pypi.python.org/pypi/isort
