darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-09-08 01:58:26 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1065 from alphagov/fix-invitation-bug

Browse Source 
Fix edge case bug in the invitation flow
This commit is contained in:
Rebecca Law
2016-12-22 15:37:05 +00:00
committed by GitHub
parent 01b9aad0fb d0a5ef0247
commit cc40f81ded
2 changed files with 94 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/main/views/register.py
View File

@@ -80,8 +80,8 @@ def _do_registration(form, service=None, send_sms=True, send_email=True):
session['expiry_date'] = str(datetime.utcnow() + timedelta(hours=1)) session['expiry_date'] = str(datetime.utcnow() + timedelta(hours=1))
session['user_details'] = {"email": user.email_address, "id": user.id} session['user_details'] = {"email": user.email_address, "id": user.id}
else: else:
user = user_api_client.get_user_by_email(form.email_address.data)
if send_email: if send_email:
user = user_api_client.get_user_by_email(form.email_address.data)
user_api_client.send_already_registered_email(user.id, user.email_address) user_api_client.send_already_registered_email(user.id, user.email_address)
session['expiry_date'] = str(datetime.utcnow() + timedelta(hours=1)) session['expiry_date'] = str(datetime.utcnow() + timedelta(hours=1))
session['user_details'] = {"email": user.email_address, "id": user.id} session['user_details'] = {"email": user.email_address, "id": user.id}

151
tests/app/main/views/test_register.py
View File

@@ -1,91 +1,86 @@
from datetime import datetime
from flask import ( from flask import (
url_for, url_for,
session session
) )
from app.notify_client.models import InvitedUser
from bs4 import BeautifulSoup
def test_render_register_returns_template_with_form(app_): def test_render_register_returns_template_with_form(client):
response = app_.test_client().get('/register') response = client.get('/register')
assert response.status_code == 200 assert response.status_code == 200
assert 'Create an account' in response.get_data(as_text=True) assert 'Create an account' in response.get_data(as_text=True)
def test_logged_in_user_redirects_to_choose_service(app_, def test_logged_in_user_redirects_to_choose_service(client,
api_user_active, api_user_active,
mock_get_user_by_email, mock_get_user_by_email,
mock_send_verify_code, mock_send_verify_code,
mock_login): mock_login):
with app_.test_request_context(): client.login(api_user_active)
with app_.test_client() as client: response = client.get(url_for('main.register'))
client.login(api_user_active) assert response.status_code == 302
response = client.get(url_for('main.register'))
assert response.status_code == 302
response = client.get(url_for('main.sign_in', follow_redirects=True)) response = client.get(url_for('main.sign_in', follow_redirects=True))
assert response.location == url_for('main.choose_service', _external=True) assert response.location == url_for('main.choose_service', _external=True)
def test_register_creates_new_user_and_redirects_to_continue_page(app_, def test_register_creates_new_user_and_redirects_to_continue_page(client,
mock_send_verify_code, mock_send_verify_code,
mock_register_user, mock_register_user,
mock_get_user_by_email_not_found, mock_get_user_by_email_not_found,
mock_is_email_unique, mock_is_email_unique,
mock_send_verify_email, mock_send_verify_email,
mock_login): mock_login):
user_data = {'name': 'Some One Valid', user_data = {'name': 'Some One Valid',
'email_address': 'notfound@example.gov.uk', 'email_address': 'notfound@example.gov.uk',
'mobile_number': '+4407700900460', 'mobile_number': '+4407700900460',
'password': 'validPassword!' 'password': 'validPassword!'
} }
with app_.test_request_context(): response = client.post(url_for('main.register'), data=user_data)
response = app_.test_client().post(url_for('main.register'), data=user_data) assert response.status_code == 302
assert response.status_code == 302 assert response.location == url_for('main.registration_continue', _external=True)
assert response.location == url_for('main.registration_continue', _external=True)
from unittest.mock import ANY from unittest.mock import ANY
mock_send_verify_email.assert_called_with(ANY, user_data['email_address']) mock_send_verify_email.assert_called_with(ANY, user_data['email_address'])
mock_register_user.assert_called_with(user_data['name'], mock_register_user.assert_called_with(user_data['name'],
user_data['email_address'], user_data['email_address'],
user_data['mobile_number'], user_data['mobile_number'],
user_data['password']) user_data['password'])
def test_process_register_returns_200_when_mobile_number_is_invalid(app_, def test_process_register_returns_200_when_mobile_number_is_invalid(client,
mock_send_verify_code, mock_send_verify_code,
mock_get_user_by_email_not_found, mock_get_user_by_email_not_found,
mock_login): mock_login):
with app_.test_request_context(): response = client.post(url_for('main.register'),
response = app_.test_client().post(url_for('main.register'), data={'name': 'Bad Mobile',
data={'name': 'Bad Mobile', 'email_address': 'bad_mobile@example.gov.uk',
'email_address': 'bad_mobile@example.gov.uk', 'mobile_number': 'not good',
'mobile_number': 'not good', 'password': 'validPassword!'})
'password': 'validPassword!'})
assert response.status_code == 200 assert response.status_code == 200
assert 'Must not contain letters or symbols' in response.get_data(as_text=True) assert 'Must not contain letters or symbols' in response.get_data(as_text=True)
def test_should_return_200_when_email_is_not_gov_uk(app_, def test_should_return_200_when_email_is_not_gov_uk(client,
mock_send_verify_code, mock_send_verify_code,
mock_get_user_by_email, mock_get_user_by_email,
mock_login): mock_login):
with app_.test_request_context(): response = client.post(url_for('main.register'),
response = app_.test_client().post(url_for('main.register'), data={'name': 'Bad Mobile',
data={'name': 'Bad Mobile', 'email_address': 'bad_mobile@example.not.right',
'email_address': 'bad_mobile@example.not.right', 'mobile_number': '+44123412345',
'mobile_number': '+44123412345', 'password': 'validPassword!'})
'password': 'validPassword!'})
assert response.status_code == 200 assert response.status_code == 200
assert 'Enter a central government email address' in response.get_data(as_text=True) assert 'Enter a central government email address' in response.get_data(as_text=True)
def test_should_add_user_details_to_session(app_, def test_should_add_user_details_to_session(client,
mock_send_verify_code, mock_send_verify_code,
mock_register_user, mock_register_user,
mock_get_user, mock_get_user,
@@ -100,30 +95,26 @@ def test_should_add_user_details_to_session(app_,
'password': 'validPassword!' 'password': 'validPassword!'
} }
with app_.test_request_context(): response = client.post(url_for('main.register'), data=user_data)
with app_.test_client() as client:
response = client.post(url_for('main.register'),
data=user_data)
assert response.status_code == 302 assert response.status_code == 302
assert session['user_details']['email'] == user_data['email_address'] assert session['user_details']['email'] == user_data['email_address']
def test_should_return_200_if_password_is_blacklisted(app_, def test_should_return_200_if_password_is_blacklisted(client,
mock_get_user_by_email, mock_get_user_by_email,
mock_login): mock_login):
with app_.test_request_context(): response = client.post(url_for('main.register'),
response = app_.test_client().post(url_for('main.register'), data={'name': 'Bad Mobile',
data={'name': 'Bad Mobile', 'email_address': 'bad_mobile@example.not.right',
'email_address': 'bad_mobile@example.not.right', 'mobile_number': '+44123412345',
'mobile_number': '+44123412345', 'password': 'password'})
'password': 'password'})
response.status_code == 200 response.status_code == 200
assert 'Choose a password that’s harder to guess' in response.get_data(as_text=True) assert 'Choose a password that’s harder to guess' in response.get_data(as_text=True)
def test_register_with_existing_email_sends_emails(app_, def test_register_with_existing_email_sends_emails(client,
api_user_active, api_user_active,
mock_get_user_by_email, mock_get_user_by_email,
mock_send_already_registered_email): mock_send_already_registered_email):
@@ -134,8 +125,52 @@ def test_register_with_existing_email_sends_emails(app_,
'password': 'validPassword!' 'password': 'validPassword!'
} }
with app_.test_request_context(): response = client.post(url_for('main.register'),
response = app_.test_client().post(url_for('main.register'), data=user_data)
data=user_data) assert response.status_code == 302
assert response.status_code == 302 assert response.location == url_for('main.registration_continue', _external=True)
assert response.location == url_for('main.registration_continue', _external=True)
def test_register_from_invite_(client,
fake_uuid,
mock_is_email_unique,
mock_register_user,
mock_send_verify_code,
mock_accept_invite):
invited_user = InvitedUser(fake_uuid, fake_uuid, "",
"invited@user.com",
["manage_users"],
"pending",
datetime.utcnow())
with client.session_transaction() as session:
session['invited_user'] = invited_user.serialize()
response = client.post(url_for('main.register_from_invite'),
data={'name': 'Registered in another Browser',
'email_address': invited_user.email_address,
'mobile_number': '+4407700900460',
'service': str(invited_user.id),
'password': 'somreallyhardthingtoguess'})
assert response.status_code == 302
assert response.location == url_for('main.verify', _external=True)
def test_register_from_invite_when_user_registers_in_another_browser(client,
api_user_active,
mock_is_email_not_unique,
mock_get_user_by_email,
mock_accept_invite):
invited_user = InvitedUser(api_user_active.id, api_user_active.id, "",
api_user_active.email_address,
["manage_users"],
"pending",
datetime.utcnow())
with client.session_transaction() as session:
session['invited_user'] = invited_user.serialize()
response = client.post(url_for('main.register_from_invite'),
data={'name': 'Registered in another Browser',
'email_address': api_user_active.email_address,
'mobile_number': api_user_active.mobile_number,
'service': str(api_user_active.id),
'password': 'somreallyhardthingtoguess'})
assert response.status_code == 302
assert response.location == url_for('main.verify', _external=True)