The data flow of other bits of our application looks like this:
```
API (returns JSON)
⬇
API client (returns a built in type, usually `dict`)
⬇
Model (returns an instance, eg of type `Service`)
⬇
View (returns HTML)
```
The user API client was architected weirdly, in that it returned a model
directly, like this:
```
API (returns JSON)
⬇
API client (returns a model, of type `User`, `InvitedUser`, etc)
⬇
View (returns HTML)
```
This mixing of different layers of the application is bad because it
makes it hard to write model code that doesn’t have circular
dependencies. As our application gets more complicated we will be
relying more on models to manage this complexity, so we should make it
easy, not hard to write them.
It also means that most of our mocking was of the User model, not just
the underlying JSON. So it would have been easy to introduce subtle bugs
to the user model, because it wasn’t being comprehensively tested. A lot
of the changed lines of code in this commit mean changing the tests to
mock only the JSON, which means that the model layer gets implicitly
tested.
For those reasons this commit changes the user API client to return
JSON, not an instance of `User` or other models.
This commit stops a new email verification link from being sent to a
user if they click on an email link which has expired or which has
already been used. Instead, they will be see an error message with a
link to the sign in page. This stops the situation where someone could
log in indefinitely (without the needing to enter their password) by
trying to use a used / expired email verification link and receiving a
valid link automatically.
This commit stops a new email verification link from being sent to a
user if they click on an email link which has expired or which has
already been used. Instead, they will be see an error message with a
link to the sign in page. This stops the situation where someone could
log in indefinitely (without the needing to enter their password) by
trying to use a used / expired email verification link and receiving a
valid link automatically.
if a user signs in again, clear their file upload data from any
aborted journeys from before, so that their cookies don't fill up
also add some temporary logging when the session starts getting full.
show_accounts_or_dashboard has logic about where you should redirect
to. If we let it do this, then that's nicer than duplicating its
logic. We found that it wasn't accounting for orgs in redirects
properly.
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
The user has 10 tries at the password, after which the account is locked.
The same is true for the verify code, the user will have 10 tries before the user account is locked.
specifically, the 2FA page when you first create an account is different to the login 2FA page
and also the 2FA page when you change your phone number is different as well
when a user enters their 2FA code, the API will store a random UUID
against them in the database - this code is then stored on the cookie
on the front end.
At the beginning of each authenticated request, we do the following
steps:
* Retrieve the user's cookie, and get the user_id from it
* Request that user's details from the database
* populate current_user with the DB model
* run the login_required decorator, which calls
current_user.is_authenticated
is_authenticated now also checks that the database model matches the
cookie for session_id. The potential states and meanings are as follows:
database | cookie | meaning
----------+--------+---------
None | None | New user, or system just been deployed.
| | Redirect to start page.
----------+--------+---------
'abc' | None | New browser (or cleared cookies). Redirect to
| | start page.
----------+--------+---------
None | 'abc' | Invalid state (cookie is set from user obj, so
| | would only happen if DB is cleared)
----------+--------+---------
'abc' | 'abc' | Same browser. Business as usual
----------+--------+---------
'abc' | 'def' | Different browser in cookie - db has been changed
| | since then. Redirect to start
* all current invocations of get_services now call get_active_services
EXCEPT for platform admin page (where we want to see inactive services
* cleaned up parameter names and unpacking (since *params is unhelpful)
* fixed incorrect kwarg name in conftest
If you’re a platform admin, you should go straight to the platform admin
page when you log in.
The all services page is just a crappier version of the same thing,
without all the stats, etc.
parts of the initial setup/login stages were throwing 500s if user
not already in process (ie: user directly navigated to url):
* /resend-email-verification
* /text-not-received
* /send-new-code
* verify
We used to do this by redirecting on the choose service page. However when we
lost the dropdown and this page also became the page for adding a new service
(in 3617f2e936) the redirect was removed.
This commit re-adds the redirect on the two factor page, so that it only happens
on first login.
So the flows are:
**Multiple services**
```
`Sign in` → `Enter two factor code` → `Choose service` → `Service dashboard`
```
**One service**
```
`Sign in` → `Enter two factor code` → `Service dashboard`
```
**No services (you’ve deleted all your services)**
`Sign in` → `Enter two factor code` → `Choose service` → `Add new service`
