mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-02-05 19:03:30 -05:00
Removed some un needed flash messages raised as bugs.
In the process found a couple of edge cases of incorrect use of invitation links by other users which are now handled.
This commit is contained in:
@@ -3,7 +3,8 @@ from flask import (
|
||||
url_for,
|
||||
session,
|
||||
flash,
|
||||
render_template
|
||||
render_template,
|
||||
abort
|
||||
)
|
||||
|
||||
|
||||
@@ -17,11 +18,18 @@ from app import (
|
||||
service_api_client
|
||||
)
|
||||
|
||||
from flask_login import current_user
|
||||
|
||||
|
||||
@main.route("/invitation/<token>")
|
||||
def accept_invite(token):
|
||||
|
||||
invited_user = invite_api_client.check_token(token)
|
||||
|
||||
if not current_user.is_anonymous() and current_user.email_address != invited_user.email_address:
|
||||
flash("You can't accept an invite for another person.")
|
||||
abort(403)
|
||||
|
||||
if invited_user.status == 'cancelled':
|
||||
from_user = user_api_client.get_user(invited_user.from_user)
|
||||
service = service_api_client.get_service(invited_user.service)['data']
|
||||
@@ -31,7 +39,6 @@ def accept_invite(token):
|
||||
|
||||
if invited_user.status == 'accepted':
|
||||
session.pop('invited_user', None)
|
||||
flash('You have already accepted this invitation', 'default')
|
||||
return redirect(url_for('main.service_dashboard', service_id=invited_user.service))
|
||||
|
||||
session['invited_user'] = invited_user.serialize()
|
||||
@@ -41,15 +48,11 @@ def accept_invite(token):
|
||||
|
||||
if existing_user:
|
||||
if existing_user in service_users:
|
||||
session.pop('invited_user', None)
|
||||
flash('You have already accepted an invitation to this service', 'default')
|
||||
invite_api_client.accept_invite(invited_user.service, invited_user.id)
|
||||
return redirect(url_for('main.service_dashboard', service_id=invited_user.service))
|
||||
else:
|
||||
user_api_client.add_user_to_service(invited_user.service,
|
||||
existing_user.id,
|
||||
invited_user.permissions)
|
||||
invite_api_client.accept_invite(invited_user.service, invited_user.id)
|
||||
return redirect(url_for('main.service_dashboard', service_id=invited_user.service))
|
||||
else:
|
||||
return redirect(url_for('main.register_from_invite'))
|
||||
|
||||
@@ -4,7 +4,8 @@ from flask import (
|
||||
url_for,
|
||||
session,
|
||||
flash,
|
||||
request
|
||||
request,
|
||||
abort
|
||||
)
|
||||
|
||||
from flask.ext.login import (
|
||||
@@ -13,26 +14,40 @@ from flask.ext.login import (
|
||||
confirm_login
|
||||
)
|
||||
|
||||
from app import (
|
||||
user_api_client,
|
||||
invite_api_client,
|
||||
service_api_client
|
||||
)
|
||||
|
||||
|
||||
from app.main import main
|
||||
|
||||
from app import (user_api_client, service_api_client)
|
||||
|
||||
|
||||
from app.main.forms import LoginForm
|
||||
|
||||
|
||||
@main.route('/sign-in', methods=(['GET', 'POST']))
|
||||
def sign_in():
|
||||
|
||||
if current_user and current_user.is_authenticated():
|
||||
return redirect(url_for('main.choose_service'))
|
||||
|
||||
form = LoginForm()
|
||||
if form.validate_on_submit():
|
||||
|
||||
user = user_api_client.get_user_by_email_or_none(form.email_address.data)
|
||||
user = _get_and_verify_user(user, form.password.data)
|
||||
if user and user.state == 'pending':
|
||||
flash("You haven't verified your email or mobile number yet.")
|
||||
return redirect(url_for('main.sign_in'))
|
||||
|
||||
if user and session.get('invited_user'):
|
||||
invited_user = session.get('invited_user')
|
||||
if user.email_address != invited_user['email_address']:
|
||||
flash("You can't accept an invite for another person.")
|
||||
session.pop('invited_user', None)
|
||||
abort(403)
|
||||
else:
|
||||
invite_api_client.accept_invite(invited_user['service'], invited_user['id'])
|
||||
if user:
|
||||
# Remember me login
|
||||
if not login_fresh() and \
|
||||
@@ -56,11 +71,6 @@ def sign_in():
|
||||
# Vague error message for login in case of user not known, locked, inactive or password not verified
|
||||
flash('Username or password is incorrect')
|
||||
|
||||
invited_user = session.get('invited_user')
|
||||
if invited_user:
|
||||
message = 'You already have an account with GOV.UK Notify. Sign in to your account to accept this invitation.'
|
||||
flash(message, 'default')
|
||||
|
||||
return render_template('views/signin.html', form=form)
|
||||
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
{% extends "withoutnav_template.html" %}
|
||||
{% block page_title %}Page not found{% endblock %}
|
||||
{% block page_title %}Forbidden{% endblock %}
|
||||
{% block maincolumn_content %}
|
||||
<div class="grid-row">
|
||||
<div class="column-two-thirds">
|
||||
|
||||
@@ -16,8 +16,7 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_,
|
||||
mock_check_invite_token,
|
||||
mock_get_user_by_email,
|
||||
mock_get_users_by_service,
|
||||
mock_add_user_to_service,
|
||||
mock_accept_invite):
|
||||
mock_add_user_to_service):
|
||||
|
||||
expected_service = service_one['id']
|
||||
expected_redirect_location = 'http://localhost/services/{}/dashboard'.format(expected_service)
|
||||
@@ -31,7 +30,6 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_,
|
||||
mock_check_invite_token.assert_called_with('thisisnotarealtoken')
|
||||
mock_get_user_by_email.assert_called_with('invited_user@test.gov.uk')
|
||||
mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id, expected_permissions)
|
||||
mock_accept_invite.assert_called_with(expected_service, sample_invite['id'])
|
||||
|
||||
assert response.status_code == 302
|
||||
assert response.location == expected_redirect_location
|
||||
@@ -61,9 +59,9 @@ def test_existing_user_with_no_permissions_accept_invite(app_,
|
||||
assert response.status_code == 302
|
||||
|
||||
|
||||
def test_existing_user_cant_accept_twice(app_,
|
||||
mocker,
|
||||
sample_invite):
|
||||
def test_if_existing_user_accepts_twice_they_redirect_to_sign_in(app_,
|
||||
mocker,
|
||||
sample_invite):
|
||||
|
||||
sample_invite['status'] = 'accepted'
|
||||
invite = InvitedUser(**sample_invite)
|
||||
@@ -76,16 +74,16 @@ def test_existing_user_cant_accept_twice(app_,
|
||||
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
||||
assert page.h1.string.strip() == 'Sign in'
|
||||
flash_banners = page.find_all('div', class_='banner-default')
|
||||
assert len(flash_banners) == 2
|
||||
assert flash_banners[0].text.strip() == 'You have already accepted this invitation'
|
||||
assert len(flash_banners) == 1
|
||||
assert flash_banners[0].text.strip() == 'Please log in to access this page.'
|
||||
|
||||
|
||||
def test_existing_of_service_get_message_that_they_are_already_part_of_service(app_,
|
||||
mocker,
|
||||
api_user_active,
|
||||
sample_invite,
|
||||
mock_get_user_by_email,
|
||||
mock_accept_invite):
|
||||
def test_existing_user_of_service_get_redirected_to_signin(app_,
|
||||
mocker,
|
||||
api_user_active,
|
||||
sample_invite,
|
||||
mock_get_user_by_email,
|
||||
mock_accept_invite):
|
||||
sample_invite['email_address'] = api_user_active.email_address
|
||||
invite = InvitedUser(**sample_invite)
|
||||
mocker.patch('app.invite_api_client.check_token', return_value=invite)
|
||||
@@ -98,8 +96,8 @@ def test_existing_of_service_get_message_that_they_are_already_part_of_service(a
|
||||
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
||||
assert page.h1.string.strip() == 'Sign in'
|
||||
flash_banners = page.find_all('div', class_='banner-default')
|
||||
assert len(flash_banners) == 2
|
||||
assert flash_banners[0].text.strip() == 'You have already accepted an invitation to this service'
|
||||
assert len(flash_banners) == 1
|
||||
assert flash_banners[0].text.strip() == 'Please log in to access this page.'
|
||||
|
||||
|
||||
def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_,
|
||||
@@ -109,8 +107,7 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_,
|
||||
mock_check_invite_token,
|
||||
mock_get_user_by_email,
|
||||
mock_get_users_by_service,
|
||||
mock_add_user_to_service,
|
||||
mock_accept_invite):
|
||||
mock_add_user_to_service):
|
||||
|
||||
expected_service = service_one['id']
|
||||
expected_permissions = ['send_messages', 'manage_service', 'manage_api_keys']
|
||||
@@ -122,15 +119,13 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_,
|
||||
mock_check_invite_token.assert_called_with('thisisnotarealtoken')
|
||||
mock_get_user_by_email.assert_called_with('invited_user@test.gov.uk')
|
||||
mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id, expected_permissions)
|
||||
mock_accept_invite.assert_called_with(expected_service, sample_invite['id'])
|
||||
|
||||
assert response.status_code == 200
|
||||
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
||||
assert page.h1.string.strip() == 'Sign in'
|
||||
flash_banners = page.find_all('div', class_='banner-default')
|
||||
assert len(flash_banners) == 2
|
||||
assert len(flash_banners) == 1
|
||||
assert flash_banners[0].text.strip() == 'Please log in to access this page.'
|
||||
assert flash_banners[1].text.strip() == 'You already have an account with GOV.UK Notify. Sign in to your account to accept this invitation.' # noqa
|
||||
|
||||
|
||||
def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_,
|
||||
@@ -138,8 +133,7 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_,
|
||||
mock_check_invite_token,
|
||||
mock_dont_get_user_by_email,
|
||||
mock_add_user_to_service,
|
||||
mock_get_users_by_service,
|
||||
mock_accept_invite):
|
||||
mock_get_users_by_service):
|
||||
|
||||
expected_redirect_location = 'http://localhost/register-from-invite'
|
||||
|
||||
@@ -160,8 +154,7 @@ def test_new_user_accept_invite_calls_api_and_views_registration_page(app_,
|
||||
mock_check_invite_token,
|
||||
mock_dont_get_user_by_email,
|
||||
mock_add_user_to_service,
|
||||
mock_get_users_by_service,
|
||||
mock_accept_invite):
|
||||
mock_get_users_by_service):
|
||||
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
@@ -219,8 +212,7 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(a
|
||||
mock_register_user,
|
||||
mock_send_verify_code,
|
||||
mock_get_users_by_service,
|
||||
mock_add_user_to_service,
|
||||
mock_accept_invite):
|
||||
mock_add_user_to_service):
|
||||
|
||||
expected_service = service_one['id']
|
||||
expected_email = sample_invite['email_address']
|
||||
@@ -261,6 +253,59 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(a
|
||||
data['password'])
|
||||
|
||||
|
||||
def test_signed_in_existing_user_cannot_use_anothers_invite(app_,
|
||||
mocker,
|
||||
api_user_active,
|
||||
sample_invite,
|
||||
mock_get_user,
|
||||
mock_accept_invite):
|
||||
invite = InvitedUser(**sample_invite)
|
||||
mocker.patch('app.invite_api_client.check_token', return_value=invite)
|
||||
mocker.patch('app.user_api_client.get_users_for_service', return_value=[api_user_active])
|
||||
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True)
|
||||
assert response.status_code == 403
|
||||
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
||||
assert page.h1.string.strip() == '403'
|
||||
flash_banners = page.find_all('div', class_='banner-dangerous')
|
||||
assert len(flash_banners) == 1
|
||||
assert flash_banners[0].text.strip() == "You can't accept an invite for another person."
|
||||
|
||||
|
||||
def test_signed_out_existing_user_cannot_use_anothers_invite(app_,
|
||||
mocker,
|
||||
api_user_active,
|
||||
sample_invite,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_verify_password,
|
||||
mock_send_verify_code,
|
||||
mock_accept_invite):
|
||||
invite = InvitedUser(**sample_invite)
|
||||
mocker.patch('app.invite_api_client.check_token', return_value=invite)
|
||||
mocker.patch('app.user_api_client.get_users_for_service', return_value=[api_user_active])
|
||||
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True)
|
||||
assert response.status_code == 200
|
||||
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
||||
assert page.h1.string.strip() == 'Sign in'
|
||||
response = client.post(url_for('main.sign_in'),
|
||||
data={'email_address': 'test@user.gov.uk', 'password': 'somepassword'},
|
||||
follow_redirects=True)
|
||||
|
||||
assert response.status_code == 403
|
||||
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
||||
assert page.h1.string.strip() == '403'
|
||||
flash_banners = page.find_all('div', class_='banner-dangerous')
|
||||
assert len(flash_banners) == 1
|
||||
assert flash_banners[0].text.strip() == "You can't accept an invite for another person."
|
||||
|
||||
|
||||
def test_new_invited_user_verifies_and_added_to_service(app_,
|
||||
service_one,
|
||||
sample_invite,
|
||||
|
||||
Reference in New Issue
Block a user