This changeset pulls in all of the notification_utils code directly into the admin and removes it as an external dependency. We are doing this to cut down on operational maintenance of the project and will begin removing parts of it no longer needed for the admin.
Signed-off-by: Carlo Costino <carlo.costino@gsa.gov>
This changeset removes webauthn from the Notify.gov admin app. We are not using webauthn at all in our implementation and will be looking at an entirely different authentication system in the near future.
Signed-off-by: Carlo Costino <carlo.costino@gsa.gov>
When someone uses a fresh password reset link they have proved that they
have access to their inbox.
At the moment, when revalidating a user’s email address we wait until
after they’ve put in the 2FA code before updating the timestamp which
records when they last validated their email address[1].
We can’t think of a good reason that we need the extra assurance of a
valid 2FA code to assert that the user has access to their email –
they’ve done that just by clicking the link. When the user clicks the
link we already update their failed login count before they 2fa. Think
it makes sense to handle `email_access_validated_at` then too.
As a bonus, the functional tests never go as far as getting a 2FA code
after a password reset[2], so the functional test user never gets its
timestamp updated. This causes the functional tests start failing after
90 days. By moving the update to this point we ensure that the
functional tests will keep passing indefinitely.
1. This code in the API (91542ad33e/app/dao/users_dao.py (L131))
which is called by this code in the admin app (9ba37249a4/app/utils/login.py (L26))
2. 5837eb01dc/tests/functional/preview_and_dev/test_email_auth.py (L43-L46)
This better reflects how the code is reused in other views and is
not specific to two factor actions. We have a pattern of testing
utility functionality for each view (as opposed to testing the util
+ the view calls the util), so I'm leaving the tests as-is.
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.
Rather than hard-coding a format string in a bunch of different places
we can use the function we already have in utils.
This commit also refactors some logic around password resets to put the
date-parsing changes in the most sensible bit of the codebase, so it’s
clearer what the intention of the view-layer code is.
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.
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
we currently store new account email verify tokens in the database, and
check against that to work out if they've expired. But we don't need to
do that, tokens have their own timing mechanism. So lets just use that,
and free up the database to do other things.
Also, standardised the forgot password, change email, and new account
email verification timeouts to all be an hour, from the config val
'EMAIL_EXPIRY_SECONDS'
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.
Which means the user will only be able to reset their password, and not sign-in.
Once the user resets the password the user state is set to active once more.
If the link is used a second time they will be redirected to the index page with a message
that the link in the email is not longer valid.
Refactored the forms so that fields like email_address can be used in multiple forms.
Refactored form validation so that a query function is passed into the form to be run, this
way the form is not exposed to the dao layer and the query is more efficient.
This PR still requires some frontend attention. Will work with Chris to update the templates.
Found a way to create the token that does not need to persist it to the database.
This requires proper error messages, written by people who speak menglis good.
Start implementation for new-password endpoints.
Created PasswordResetToken model
ToDo: create and save token, send valid url to user,
check validity of token, update user's password, redirect to /two-factor.
