darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-09-02 17:48:50 -04:00
Code Issues Packages Projects Releases Wiki Activity
Files
f8fb2d3c8f010bb9fe49495ff279cea06cd5635c
notifications-admin/tests/app/main/views/test_sign_in.py

212 lines
7.0 KiB
Python
Raw Normal View History

check users' session id. 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
2017-02-17 14:06:09 +00:00
import uuid
import pytest
If user is pending it means they have not verified email yet Added better checking on re use of consumed verification link.
2016-03-29 12:13:36 +01:00
from bs4 import BeautifulSoup
Enforce order and style of imports 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
2018-02-20 11:22:17 +00:00
from flask import url_for
Fixed some tests. Some broken mocked tests commented out until later
2016-01-21 12:31:09 +00:00
Make user API client return JSON, not a model 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.
2019-05-23 15:27:35 +01:00
from app.models.user import User
108536490: Initial effort to implement log in Add endpoint for post to /sign-in Initialise role data
2015-11-27 09:47:29 +00:00
check users' session id. 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
2017-02-17 14:06:09 +00:00
def test_render_sign_in_template_for_new_user(
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
client
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
):
check users' session id. 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
2017-02-17 14:06:09 +00:00
response = client.get(url_for('main.sign_in', next=None))
assert response.status_code == 200
resp = response.get_data(as_text=True)
assert 'Sign in' in resp
assert 'Email address' in resp
assert 'Password' in resp
assert 'Forgot your password?' in resp
assert 'If you do not have an account, you can' in resp
assert 'Sign in again' not in resp
def test_sign_in_explains_session_timeout(client):
response = client.get(url_for('main.sign_in', next='/foo'))
108536490: Initial effort to implement log in Add endpoint for post to /sign-in Initialise role data
2015-11-27 09:47:29 +00:00
assert response.status_code == 200
check users' session id. 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
2017-02-17 14:06:09 +00:00
assert 'We signed you out because you haven’t used Notify for a while.' in response.get_data(as_text=True)
def test_sign_in_explains_other_browser(logged_in_client, api_user_active, mocker):
Make user API client return JSON, not a model 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.
2019-05-23 15:27:35 +01:00
api_user_active['current_session_id'] = str(uuid.UUID(int=1))
check users' session id. 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
2017-02-17 14:06:09 +00:00
mocker.patch('app.user_api_client.get_user', return_value=api_user_active)
with logged_in_client.session_transaction() as session:
session['current_session_id'] = str(uuid.UUID(int=2))
response = logged_in_client.get(url_for('main.sign_in', next='/foo'))
assert response.status_code == 200
assert 'We signed you out because you logged in to Notify on another device' in response.get_data(as_text=True)
rename choose_service to choose_account
2018-03-08 16:51:53 +00:00
def test_doesnt_redirect_to_sign_in_if_no_session_info(
Don’t ask for organisation type when we know it Every time someone adds a new service we ask them what kind of organisation they work for. We can look this up based on the user’s email address now. So we should only ask the question if: - we don’t know about the organisation - or we haven’t set what type of organisation it is (this shouldn’t be possible on productions because we’ve populated the column for all existing organisations and it’s impossible to add a new one without setting it
2019-04-18 12:44:18 +01:00
client_request,
api_user_active,
mock_get_organisation_by_domain,
rename choose_service to choose_account
2018-03-08 16:51:53 +00:00
):
Make user API client return JSON, not a model 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.
2019-05-23 15:27:35 +01:00
assert 'current_session_id' not in api_user_active
rename choose_service to choose_account
2018-03-08 16:51:53 +00:00
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
with client_request.session_transaction() as session:
check users' session id. 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
2017-02-17 14:06:09 +00:00
session['current_session_id'] = None
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request.get('main.add_service')
check users' session id. 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
2017-02-17 14:06:09 +00:00
@pytest.mark.parametrize('db_sess_id, cookie_sess_id', [
don't apply marks directly in pytest specifically - don't use `pytest.mark.xfail` directly in parametrize, instead use `pytest.param(*args, marks=pytest.mark.xfail)`. the old way is deprecated in pytest4 - for more information see https://docs.pytest.org/en/latest/deprecations.html#marks-in-pytest-mark-parametrize Also, make this an error in pytest.ini so if someone adds a new xfail, it'll crash
2018-12-31 13:10:19 +00:00
pytest.param(None, None, marks=pytest.mark.xfail), # OK - not used notify since browser signout was implemented
check users' session id. 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
2017-02-17 14:06:09 +00:00
(uuid.UUID(int=1), None), # BAD - has used other browsers before but this is a brand new browser with no cookie
(uuid.UUID(int=1), uuid.UUID(int=2)), # BAD - this person has just signed in on a different browser
])
def test_redirect_to_sign_in_if_logged_in_from_other_browser(
logged_in_client,
api_user_active,
mocker,
db_sess_id,
cookie_sess_id
):
Make user API client return JSON, not a model 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.
2019-05-23 15:27:35 +01:00
api_user_active['current_session_id'] = db_sess_id
check users' session id. 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
2017-02-17 14:06:09 +00:00
mocker.patch('app.user_api_client.get_user', return_value=api_user_active)
with logged_in_client.session_transaction() as session:
session['current_session_id'] = str(cookie_sess_id)
rename choose_service to choose_account
2018-03-08 16:51:53 +00:00
response = logged_in_client.get(url_for('main.choose_account'))
check users' session id. 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
2017-02-17 14:06:09 +00:00
assert response.status_code == 302
rename choose_service to choose_account
2018-03-08 16:51:53 +00:00
assert response.location == url_for('main.sign_in', next='/accounts', _external=True)
108536490: Initial effort to implement log in Add endpoint for post to /sign-in Initialise role data
2015-11-27 09:47:29 +00:00
Redirect signed-in users to their service 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.
2018-11-15 15:38:43 +00:00
def test_logged_in_user_redirects_to_account(
client_request
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
):
Redirect signed-in users to their service 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.
2018-11-15 15:38:43 +00:00
client_request.get(
'main.sign_in',
_expected_status=302,
_expected_redirect=url_for('main.show_accounts_or_dashboard', _external=True),
)
If user is logged in and visits / /sign-in or /register they will be redirected to choose service.
2016-01-22 17:24:14 +00:00
Remove existing whitespace stripping code 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.
2017-12-11 16:09:19 +00:00
@pytest.mark.parametrize('email_address, password', [
('valid@example.gov.uk', 'val1dPassw0rd!'),
(' valid@example.gov.uk ', ' val1dPassw0rd! '),
Strip spaces from email addresses when signing in If you copy and paste an email address into the sign in box, there’s a chance you’ll also copy some leading or trailing spaces. This is especially likely to happen if you’re doing this while using your computer upside down. If this happens, it never even gets as far as looking up the user, because the form validation doesn’t consider a string with a leading space to be a valid email address. This commit makes sure that accidental spaces are handled, by removing them before doing any validation or hitting the API to look up the user.
2017-12-06 20:24:25 +00:00
])
alter login flow to allow for email auth login
2017-11-07 16:11:31 +00:00
def test_process_sms_auth_sign_in_return_2fa_template(
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
client,
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
api_user_active,
mock_send_verify_code,
mock_get_user,
mock_get_user_by_email,
mock_verify_password,
Strip spaces from email addresses when signing in If you copy and paste an email address into the sign in box, there’s a chance you’ll also copy some leading or trailing spaces. This is especially likely to happen if you’re doing this while using your computer upside down. If this happens, it never even gets as far as looking up the user, because the form validation doesn’t consider a string with a leading space to be a valid email address. This commit makes sure that accidental spaces are handled, by removing them before doing any validation or hitting the API to look up the user.
2017-12-06 20:24:25 +00:00
email_address,
Remove existing whitespace stripping code 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.
2017-12-11 16:09:19 +00:00
password,
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
):
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
response = client.post(
url_for('main.sign_in'), data={
Strip spaces from email addresses when signing in If you copy and paste an email address into the sign in box, there’s a chance you’ll also copy some leading or trailing spaces. This is especially likely to happen if you’re doing this while using your computer upside down. If this happens, it never even gets as far as looking up the user, because the form validation doesn’t consider a string with a leading space to be a valid email address. This commit makes sure that accidental spaces are handled, by removing them before doing any validation or hitting the API to look up the user.
2017-12-06 20:24:25 +00:00
'email_address': email_address,
Remove existing whitespace stripping code 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.
2017-12-11 16:09:19 +00:00
'password': password})
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
assert response.status_code == 302
assert response.location == url_for('.two_factor', _external=True)
Make user API client return JSON, not a model 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.
2019-05-23 15:27:35 +01:00
mock_verify_password.assert_called_with(api_user_active['id'], password)
Strip spaces from email addresses when signing in If you copy and paste an email address into the sign in box, there’s a chance you’ll also copy some leading or trailing spaces. This is especially likely to happen if you’re doing this while using your computer upside down. If this happens, it never even gets as far as looking up the user, because the form validation doesn’t consider a string with a leading space to be a valid email address. This commit makes sure that accidental spaces are handled, by removing them before doing any validation or hitting the API to look up the user.
2017-12-06 20:24:25 +00:00
mock_get_user_by_email.assert_called_with('valid@example.gov.uk')
108536490: Adding the login manager and csrf token. Still need to figure out how to override the load_user method, currently it is not working.
2015-11-27 16:25:56 +00:00
alter login flow to allow for email auth login
2017-11-07 16:11:31 +00:00
def test_process_email_auth_sign_in_return_2fa_template(
client,
api_user_active_email_auth,
mock_send_verify_code,
mock_verify_password,
mocker
):
mocker.patch('app.user_api_client.get_user', return_value=api_user_active_email_auth)
mocker.patch('app.user_api_client.get_user_by_email', return_value=api_user_active_email_auth)
response = client.post(
url_for('main.sign_in'), data={
'email_address': 'valid@example.gov.uk',
'password': 'val1dPassw0rd!'})
assert response.status_code == 302
assert response.location == url_for('.two_factor_email_sent', _external=True)
Make user API client return JSON, not a model 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.
2019-05-23 15:27:35 +01:00
mock_send_verify_code.assert_called_with(api_user_active_email_auth['id'], 'email', None, None)
mock_verify_password.assert_called_with(api_user_active_email_auth['id'], 'val1dPassw0rd!')
alter login flow to allow for email auth login
2017-11-07 16:11:31 +00:00
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
def test_should_return_locked_out_true_when_user_is_locked(
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
client,
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
mock_get_user_by_email_locked,
):
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
resp = client.post(
url_for('main.sign_in'), data={
'email_address': 'valid@example.gov.uk',
'password': 'whatIsMyPassword!'})
assert resp.status_code == 200
assert 'The email address or password you entered is incorrect' in resp.get_data(as_text=True)
108536490: Add test that post with bad password when account is locked results in 401
2015-11-30 16:52:28 +00:00
108536490: Implement locked out function. User is locked if they fail to login 10 times or more.
2015-11-30 16:33:45 +00:00
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
def test_should_return_200_when_user_does_not_exist(
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
client,
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
mock_get_user_by_email_not_found,
):
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
response = client.post(
url_for('main.sign_in'), data={
'email_address': 'notfound@gov.uk',
'password': 'doesNotExist!'})
If user is logged in and visits / /sign-in or /register they will be redirected to choose service.
2016-01-22 17:24:14 +00:00
assert response.status_code == 200
Updated to include corrected flash message.
2016-04-26 12:26:41 +01:00
assert 'The email address or password you entered is incorrect' in response.get_data(as_text=True)
If user is logged in and visits / /sign-in or /register they will be redirected to choose service.
2016-01-22 17:24:14 +00:00
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
def test_should_return_redirect_when_user_is_pending(
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
client,
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
mock_get_user_by_email_pending,
mock_verify_password,
):
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
response = client.post(
url_for('main.sign_in'), data={
'email_address': 'pending_user@example.gov.uk',
'password': 'val1dPassw0rd!'}, follow_redirects=True)
Redirect and resend verification email when pending user attempts to login
2016-09-06 15:44:33 +01:00
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string == 'Sign in'
assert response.status_code == 200
Redirect and resend verification email when pending user attempts to login
2016-09-06 15:44:33 +01:00
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
def test_should_attempt_redirect_when_user_is_pending(
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
client,
Normalize whitespace in test arguments 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.
2017-02-03 10:42:01 +00:00
mock_get_user_by_email_pending,
mock_verify_password,
):
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
response = client.post(
url_for('main.sign_in'), data={
'email_address': 'pending_user@example.gov.uk',
'password': 'val1dPassw0rd!'})
assert response.location == url_for('main.resend_email_verification', _external=True)
assert response.status_code == 302
Make email addresses case insensitive when inviting users to services Email addresses in invites should be case insensitive. This is to stop the bug where a user creates their account using a lower case email address (e.g. user1@gov.uk), but is then invited to a service using their email address in a different case (e.g. USER1.gov.uk) and sees an error message telling them that they can't accept an invite for a different email address.
2017-12-21 16:42:16 +00:00
def test_email_address_is_treated_case_insensitively_when_signing_in_as_invited_user(
client,
mocker,
mock_verify_password,
api_user_active,
sample_invite,
mock_accept_invite,
mock_send_verify_code
):
sample_invite['email_address'] = 'TEST@user.gov.uk'
Make user API client return JSON, not a model 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.
2019-05-23 15:27:35 +01:00
mocker.patch(
'app.models.user.User.from_email_address_and_password_or_none',
return_value=User(api_user_active),
)
Make email addresses case insensitive when inviting users to services Email addresses in invites should be case insensitive. This is to stop the bug where a user creates their account using a lower case email address (e.g. user1@gov.uk), but is then invited to a service using their email address in a different case (e.g. USER1.gov.uk) and sees an error message telling them that they can't accept an invite for a different email address.
2017-12-21 16:42:16 +00:00
with client.session_transaction() as session:
session['invited_user'] = sample_invite
response = client.post(
url_for('main.sign_in'), data={
'email_address': 'test@user.gov.uk',
'password': 'val1dPassw0rd!'})
assert mock_accept_invite.called
assert response.status_code == 302
assert mock_send_verify_code.called
Reference in New Issue Copy Permalink