At the moment we have to update a YAML file and deploy the change to get
a new domain whitelisted.
We already have a thing for adding new domains – the organisation stuff.
This commit extends the validation to look in the `domains` table on the
API if it can’t find anything in the YAML whitelist.
This has the advantage of:
- not having to deploy code to whitelist a new domain
- forcing us to create new organisations as they come along, so that
users’ services automatically get allocated to the organisation once
their domain is whitelisted
We were already invitializing InvitedUser with folder_permissions
(defaulting to None), but this removes the default and adds
folder_permissions to the serialize method. Folder permissions should
now always be returned from api, either as an empty list or a list of
UUIDs.
We were adding invited users to services in the `main.add_service` view
function as the last step in the process of inviting users. Since this
view function is decorated with `@user_is_gov_user`, invited users with
non-governmental email addresses would never reach this point and would
be able to register an account but would not get linked to a service.
To fix this, we now add the invited user to the service at the point at
which the user gets activated and also ensure that non-gov users don't
get redirected to a page which they don't have permission to view.
Some users are bookmarking the sign in page for quick access to their
Notify service.
If they’re already signed in when they used this bookmark they get
redirected. However the redirect isn’t very smart, because it just takes
them to their list of services. This isn’t great if they only have one
service.
This commit redirects them to the `show_accounts_or_dashboard` endpoint
instead, which does the work to figure out which page is most useful
to show someone when they sign in.
Also changing this for `/register`, although it’s less likely that
someone will have bookmarked this page.
This commit is the first step to disentangling the models from the API
clients. With the models in the same folder as the API clients it makes
it hard to import the API clients within the model without getting a
circular import.
After this commit the user API clients still has this problem, but at
least the service API client doesn’t.
Things we’ve noticed from looking at real data that we could handle in a
smarter way:
- removing numbers (there might be a tom.smith2@dept.gov.uk if tom.smith
is already taken)
- removing middle initials (again, these tend to be used for
disambiguation and aren’t included when we ask people for their names)
- ignoring email addresses which only have someone’s initial, not their
first name (because we can’t make a decent guess in this case)
Most people’s names, especially in government are in the format
firstname.lastname@department.gov.uk. This means that you can pretty
reliably guess that their name is ‘Firstname Lastname’.
When users are invited to Notify we know their email address already.
So this commit pre-populates the registration form based on this guess.
This is a nice little detail, but it should also stop the browser
pre-filling the name field with someone’s email address (which I think
happens because the browser assumes a registration form will have an
email field).
tests weren't patching out create_event (which is invoked every time a
user logs in). This was getting caught by our egress proxy on jenkins.
We didn't notice because the event handler code was swallowing all
exceptions and not re-raising.
This changes that code to no longer swallow exceptions. Since we did
that, we also need to update all the tests that test log-in to mock
the call
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
Since we’re doing this globally, we don’t need to handle it in a custom
way for the sign in form (and it’s much nicer encapsulated like this).
Also added some more extensive tests in this commit.
Right now Notify restricts you to registering with a UK mobile number.
This is because when we built the user registration stuff we couldn’t
send to international mobiles.
However we can send to international mobile numbers, and it’s totally
reasonable to expect employees of the UK government to be working
abroad, and have a foreign mobile phone – we’ve heard from one such
user.
So this commit:
- changes all places where users enter their own phone number to use
the validation function which allows international phone numbers
- renames the `mobile_number` validation to `uk_mobile_number` to make
it explicit, and force it to break the tests if there’s somewhere it’s
being used that I haven’t thought of
If you go back to this part of the registration flow then you get a 500
error, because we’re relying on something in the session. We clear the
registration info from the session after you’ve registered successfully.
Also there were no tests for the happy path of this page either
¯\_(ツ)_/¯
We have a bunch of different styles of handling when function
definitions span multiple lines, which they almost always do with tests.
Here’s why an argument per line, single indent is best:
- cleaner diffs when you change the name of a method (one line change
instead of multiple lines)
- works better on narrow screens, eg Github’s diff view, or with two
terminals side by side on a laptop screen
- works with any editor’s indenting shortcuts, no need for an IDE
Also, trailing comma in the list of arguments is good because adding a
new argument to a method becomes a one line, not two line diff.
The flow is as follows:
If the invited user clicks on the /invitation/<token> link in the email (now on /register-from-invite),
then goes to another browser and registers for Notify.
Coming back to the other browser, submit the form for /register-from-invite.
This PR fixes that bug and adds a couple tests for register_from_invite
If a user chooses a very common password then an attacker could guess it
in relatively few attempts, circumventing the lockout.
CESG recommend blacklisting the most common passwords:
> …enforcing the requirement for complex character sets in passwords is
> not recommended. Instead, concentrate efforts on technical controls,
> especially:
>
> - defending against automated guessing attacks by either using account
> lockout, throttling, or protective monitoring
> - blacklisting the most common password choices
How I made this list:
- went to the OWASP repository of security lists:
https://github.com/danielmiessler/SecLists
- downloaded `10k_most_common.txt`, `twitter-banned.txt` and
`500-worst-passwords.txt`
- filtered out any under 8 characters:
```
sed -r '/^.{,7}$/d' passwords-twitter.txt > passwords-combined.txt
sed -r '/^.{,7}$/d' passwords-500.txt >> passwords-combined.txt
sed -r '/^.{,7}$/d' passwords.txt >> passwords-combined.txt
```
- filtered out any duplicates:
```
cat passwords-combined.txt | awk '!x[$0]++' > passwords-combined-deduped.txt
```
when visited sends sms code for second step of account verification.
At that second step user enters just sms code sent to users mobile
number.
Also moved dao calls that simply proxied calls to client to calling
client directly.
There is still a place where a user will be a sent a code for
verification to their email namely if they update email address.
Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9
This commit strips out a lot of the complex stuff that the views and templates
in this app were doing. There is now a cleaner separation of concerns:
- utils returns the number and type of errors in the csv
- `get_errors_for_csv` helper in this app maps the number and type of errors
onto human-friendly error messages
- the view and template just doing the glueing-together of all the pieces
This is (hopefully) easier to understand, definitely makes the component
parts easier to test in isolation, and makes it easier to give more specific
error messages.
This commit replaces the previous `StringField` used for collecting mobile
phone numbers with the `UKMobileNumber` field.
This means changing a few of the preexisting tests to have more realistic mobile
numbers so that they still pass.
