diff --git a/app/__init__.py b/app/__init__.py index 978834772..b9e070318 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -56,6 +56,7 @@ def create_app(config_name, config_overrides=None): login_manager.init_app(application) login_manager.login_view = 'main.sign_in' + login_manager.login_message_category = 'default' from app.main import main as main_blueprint application.register_blueprint(main_blueprint) diff --git a/app/main/forms.py b/app/main/forms.py index 7bb000cc7..ac5ef2f72 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -8,7 +8,8 @@ from wtforms import ( TextAreaField, FileField, RadioField, - BooleanField + BooleanField, + HiddenField ) from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import DataRequired, Email, Length, Regexp @@ -97,6 +98,15 @@ class RegisterUserForm(Form): password = password() +class RegisterUserFromInviteForm(Form): + name = StringField('Full name', + validators=[DataRequired(message='Name can not be empty')]) + mobile_number = mobile_number() + password = password() + service = HiddenField('service') + email_address = HiddenField('email_address') + + class InviteUserForm(Form): email_address = email_address('Their email address') diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 9cee69b8a..06ed495ab 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -1,15 +1,31 @@ -import re +from flask import ( + render_template, + redirect, + session, + url_for +) + +from flask_login import login_required -from flask import render_template, request, redirect, session, url_for -from flask_login import login_required, current_user from app.main import main from app.main.dao import services_dao, users_dao from app.main.forms import AddServiceForm +from app import user_api_client @main.route("/add-service", methods=['GET', 'POST']) @login_required def add_service(): + + invited_user = session.get('invited_user') + if invited_user: + # if invited user add to service and redirect to dashboard + user = users_dao.get_user_by_id(session['user_id']) + service_id = invited_user['service'] + user_api_client.add_user_to_service(service_id, user.id) + session.pop('invited_user', None) + return redirect(url_for('main.service_dashboard', service_id=service_id)) + form = AddServiceForm(services_dao.find_all_service_names) heading = 'Which service do you want to set up notifications for?' if form.validate_on_submit(): diff --git a/app/main/views/index.py b/app/main/views/index.py index 8335284cd..5f7d700df 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -12,12 +12,6 @@ def index(): return render_template('views/signedout.html') -@main.route("/register-from-invite") -@login_required -def register_from_invite(): - return render_template('views/register-from-invite.html') - - @main.route("/verify-mobile") @login_required def verify_mobile(): diff --git a/app/main/views/invites.py b/app/main/views/invites.py index c9052303d..8bd6a94da 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -1,7 +1,8 @@ from flask import ( redirect, url_for, - abort + abort, + session ) from notifications_python_client.errors import HTTPError @@ -15,6 +16,7 @@ from app import ( @main.route("/invitation/") def accept_invite(token): + try: invited_user = invite_api_client.accept_invite(token) existing_user = user_api_client.get_user_by_email(invited_user.email_address) @@ -22,11 +24,11 @@ def accept_invite(token): if existing_user: user_api_client.add_user_to_service(invited_user.service, existing_user.id) - return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) else: - # TODO implement registration flow for new users - abort(404) + session['invited_user'] = invited_user.serialize() + return redirect(url_for('main.register_from_invite')) + except HTTPError as e: if e.status_code == 404: abort(404) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index f5d83bfd6..513bfbdaf 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -4,8 +4,7 @@ from flask import ( redirect, abort, url_for, - flash -) + flash) from flask_login import ( login_required, @@ -79,7 +78,7 @@ def invite_user(service_id): @main.route("/services//users/", methods=['GET', 'POST']) @login_required -def edit_user(service_id, user_id): +def edit_user_permissions(service_id, user_id): if request.method == 'POST': return redirect(url_for('.manage_users', service_id=service_id)) @@ -116,6 +115,13 @@ def delete_user(service_id, user_id): ) +@main.route("/services//cancel-invited-user/", methods=['GET']) +def cancel_invited_user(service_id, invited_user_id): + invite_api_client.cancel_invited_user(service_id=service_id, invited_user_id=invited_user_id) + + return redirect(url_for('main.manage_users', service_id=service_id)) + + def _get_permissions(form): permissions = [] if form.get('send_messages') and form['send_messages'] == 'yes': diff --git a/app/main/views/register.py b/app/main/views/register.py index b364630d6..7400f113d 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -15,7 +15,10 @@ from notifications_python_client.errors import HTTPError from app.main import main from app.main.dao import users_dao -from app.main.forms import RegisterUserForm +from app.main.forms import ( + RegisterUserForm, + RegisterUserFromInviteForm +) from app import user_api_client @@ -27,29 +30,53 @@ def register(): form = RegisterUserForm() if form.validate_on_submit(): - if users_dao.is_email_unique(form.email_address.data): - try: - user = user_api_client.register_user(form.name.data, - form.email_address.data, - form.mobile_number.data, - form.password.data) - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + return _do_registration(form) + else: + return render_template('views/register.html', form=form) - # TODO possibly there should be some exception handling - # for sending sms and email codes. - # How do we report to the user there is a problem with - # sending codes apart from service unavailable? - # at the moment i believe http 500 is fine. - users_dao.send_verify_code(user.id, 'sms', user.mobile_number) - users_dao.send_verify_code(user.id, 'email', user.email_address) - session['expiry_date'] = str(datetime.now() + timedelta(hours=1)) - session['user_details'] = {"email": user.email_address, "id": user.id} - return redirect(url_for('main.verify')) - else: - flash('There was an error registering your account') - return render_template('views/register.html', form=form) +@main.route('/register-from-invite', methods=['GET', 'POST']) +def register_from_invite(): + + form = RegisterUserFromInviteForm() + invited_user = session.get('invited_user') + if not invited_user: + abort(404) + + if form.validate_on_submit(): + if form.service.data != invited_user['service'] or form.email_address.data != invited_user['email_address']: + abort(400) + return _do_registration(form) + + form.service.data = invited_user['service'] + form.email_address.data = invited_user['email_address'] + + return render_template('views/register-from-invite.html', form=form) + + +def _do_registration(form, service=None): + if users_dao.is_email_unique(form.email_address.data): + try: + user = user_api_client.register_user(form.name.data, + form.email_address.data, + form.mobile_number.data, + form.password.data) + + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + + # TODO possibly there should be some exception handling + # for sending sms and email codes. + # How do we report to the user there is a problem with + # sending codes apart from service unavailable? + # at the moment i believe http 500 is fine. + users_dao.send_verify_code(user.id, 'sms', user.mobile_number) + users_dao.send_verify_code(user.id, 'email', user.email_address) + session['expiry_date'] = str(datetime.now() + timedelta(hours=1)) + session['user_details'] = {"email": user.email_address, "id": user.id} + return redirect(url_for('main.verify')) + else: + flash('There was an error registering your account') diff --git a/app/main/views/send.py b/app/main/views/send.py index 1c3e9f206..b622d526a 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -159,10 +159,17 @@ def send_message_to_self(service_id, template_id): [first_column_heading[template.template_type]] + list(template.placeholders) ) - writer.writerow( - [current_user.mobile_number] + - ["test {}".format(header) for header in template.placeholders] - ) + if template.template_type == 'sms': + writer.writerow( + [current_user.mobile_number] + + ["test {}".format(header) for header in template.placeholders] + ) + if template.template_type == 'email': + writer.writerow( + [current_user.email_address] + + ["test {}".format(header) for header in template.placeholders] + ) + filedata = { 'file_name': 'Test run', 'data': output.getvalue().splitlines() diff --git a/app/main/views/verify.py b/app/main/views/verify.py index 6fda81ee4..63cc1cb18 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -17,6 +17,7 @@ from app.main.forms import VerifyForm @main.route('/verify', methods=['GET', 'POST']) def verify(): + # TODO there needs to be a way to regenerate a session id # or handle gracefully. user_id = session['user_details']['id'] diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index d1acd5f6d..6f2712848 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -1,3 +1,4 @@ + from notifications_python_client.base import BaseAPIClient from app.notify_client.models import InvitedUser @@ -27,17 +28,21 @@ class InviteApiClient(BaseAPIClient): endpoint = '/service/{}/invite'.format(service_id) resp = self.get(endpoint) invites = resp['data'] - invited_users = _get_invited_users(invites) + invited_users = self._get_invited_users(invites) return invited_users def accept_invite(self, token): resp = self.get(url='/invite/{}'.format(token)) return InvitedUser(**resp['data']) + def cancel_invited_user(self, service_id, invited_user_id): + data = {'status': 'cancelled'} + self.post(url='/service/{0}/invite/{1}'.format(service_id, invited_user_id), + data=data) -def _get_invited_users(invites): - invited_users = [] - for invite in invites: - invited_user = InvitedUser(**invite) - invited_users.append(invited_user) - return invited_users + def _get_invited_users(self, invites): + invited_users = [] + for invite in invites: + invited_user = InvitedUser(**invite) + invited_users.append(invited_user) + return invited_users diff --git a/app/notify_client/models.py b/app/notify_client/models.py index b881e8390..df8d41306 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -132,3 +132,12 @@ class InvitedUser(object): def has_permissions(self, permission): return permission in self.permissions + + def serialize(self): + return {'id': self.id, + 'service': self.service, + 'from_user': self.from_user, + 'email_address': self.email_address, + 'permissions': self.permissions, + 'status': self.status + } diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index d6a4ad574..6b3450503 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -34,8 +34,14 @@ class UserApiClient(BaseAPIClient): return User(user_data['data'], max_failed_login_count=self.max_failed_login_count) def get_user_by_email(self, email_address): - params = {'email': email_address} - user_data = self.get('/user/email', params=params) + try: + params = {'email': email_address} + user_data = self.get('/user/email', params=params) + except HTTPError as e: + if e.status_code == 404: + return None + else: + raise e return User(user_data['data'], max_failed_login_count=self.max_failed_login_count) def get_users(self): diff --git a/app/templates/views/check.html b/app/templates/views/check.html index d60bef88f..983f13175 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -49,7 +49,7 @@ {% if upload_result.valid %}
- + Back
{% else %} diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 44a0b39e5..161f966ec 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -31,9 +31,6 @@ Manage users – GOV.UK Notify {{ boolean_field(item.has_permissions(service_id, 'send_messages')) }} {{ boolean_field(item.has_permissions(service_id, 'manage_service')) }} {{ boolean_field(item.has_permissions(service_id, 'manage_api_keys')) }} - {% call field(align='right') %} - Change - {% endcall %} {% endcall %} {% if invited_users %} @@ -46,9 +43,15 @@ Manage users – GOV.UK Notify {{ boolean_field(item.has_permissions('send_messages')) }} {{ boolean_field(item.has_permissions('manage_service')) }} {{ boolean_field(item.has_permissions('manage_api_keys')) }} - {% call field(align='right') %} - Change - {% endcall %} + {% if item.status == 'pending' %} + {% call field(align='right') %} + Cancel invitation + {% endcall %} + {% else %} + {% call field() %} + {{ item.status }} + {% endcall %} + {% endif %} {% endcall %} {% endif %} diff --git a/app/templates/views/register-from-invite.html b/app/templates/views/register-from-invite.html index 4e1b14eaf..7e7e81aaa 100644 --- a/app/templates/views/register-from-invite.html +++ b/app/templates/views/register-from-invite.html @@ -1,4 +1,6 @@ {% extends "withoutnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} {% block page_title %} Create an account – GOV.UK Notify @@ -9,26 +11,14 @@ Create an account – GOV.UK Notify

Create an account

- -

If you've used GOV.UK Notify before, sign in to your account.

- -

- - -

-

- - -

-

- -
- Your password must have at least 10 characters -

- -

- Continue -

+
+ {{ textbox(form.name, width='3-4') }} + {{ textbox(form.mobile_number, width='3-4') }} + {{ textbox(form.password, hint="Your password must have at least 10 characters", width='3-4') }} + {{ page_footer("Continue") }} + {{form.service}} + {{form.email_address}} +
diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 54b527d93..78cdc28ed 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -1,9 +1,6 @@ from flask import url_for -from app.notify_client.models import InvitedUser -from notifications_python_client.errors import HTTPError - -import pytest +from bs4 import BeautifulSoup def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_, @@ -28,3 +25,142 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_, assert response.status_code == 302 assert response.location == expected_redirect_location + + +def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_, + service_one, + api_user_active, + sample_invite, + mock_accept_invite, + mock_get_user_by_email, + mock_add_user_to_service): + + expected_service = service_one['id'] + expected_redirect_location = 'http://localhost/services/{}/dashboard'.format(expected_service) + + with app_.test_request_context(): + with app_.test_client() as client: + + response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) + + mock_accept_invite.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) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Sign in' + + +def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, + service_one, + sample_invite, + mock_accept_invite, + mock_dont_get_user_by_email, + mock_add_user_to_service): + + expected_service = service_one['id'] + expected_redirect_location = 'http://localhost/register-from-invite' + + with app_.test_request_context(): + with app_.test_client() as client: + + response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + + mock_accept_invite.assert_called_with('thisisnotarealtoken') + mock_dont_get_user_by_email.assert_called_with('invited_user@test.gov.uk') + + assert response.status_code == 302 + assert response.location == expected_redirect_location + + +def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(app_, + service_one, + sample_invite, + mock_accept_invite, + mock_dont_get_user_by_email, + mock_register_user, + mock_send_verify_code, + mock_add_user_to_service): + + expected_service = service_one['id'] + expected_email = sample_invite['email_address'] + expected_from_user = service_one['users'][0] + expected_redirect_location = 'http://localhost/register-from-invite' + + with app_.test_request_context(): + with app_.test_client() as client: + response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + with client.session_transaction() as session: + assert response.status_code == 302 + assert response.location == expected_redirect_location + invited_user = session.get('invited_user') + assert invited_user + assert expected_service == invited_user['service'] + assert expected_email == invited_user['email_address'] + assert expected_from_user == invited_user['from_user'] + + data = {'service': invited_user['service'], + 'email_address': invited_user['email_address'], + 'from_user': invited_user['from_user'], + 'password': 'longpassword', + 'mobile_number': '+447890123456', + 'name': 'Invited User' + } + + expected_redirect_location = 'http://localhost/verify' + response = client.post(url_for('main.register_from_invite'), data=data) + assert response.status_code == 302 + assert response.location == expected_redirect_location + + mock_register_user.assert_called_with(data['name'], + data['email_address'], + data['mobile_number'], + data['password']) + + +def test_new_invited_user_verifies_and_added_to_service(app_, + service_one, + sample_invite, + mock_accept_invite, + mock_dont_get_user_by_email, + mock_register_user, + mock_send_verify_code, + mock_check_verify_code, + mock_get_user, + mock_update_user, + mock_add_user_to_service, + mock_get_service, + mock_get_service_templates, + mock_get_jobs): + + with app_.test_request_context(): + with app_.test_client() as client: + # visit accept token page + response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + data = {'service': sample_invite['service'], + 'email_address': sample_invite['email_address'], + 'from_user': sample_invite['from_user'], + 'password': 'longpassword', + 'mobile_number': '+447890123456', + 'name': 'Invited User' + } + + # get redirected to register from invite + response = client.post(url_for('main.register_from_invite'), data=data) + + # that sends user on to verify + response = client.post(url_for('main.verify'), data={'sms_code': '12345', 'email_code': '23456'}, + follow_redirects=True) + + # when they post codes back to admin user should be added to + # service and sent on to dash board + with client.session_transaction() as session: + new_user_id = session['user_id'] + mock_add_user_to_service.assert_called_with(data['service'], new_user_id) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + element = page.find('h2', class_='navigation-service-name').find('a') + assert element.text == 'Test Service' + service_link = element.attrs['href'] + assert service_link == '/services/{}/dashboard'.format(service_one['id']) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 438e3060e..0d4ecf093 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -30,7 +30,7 @@ def test_should_show_page_for_one_user( with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) - response = client.get(url_for('main.edit_user', service_id=55555, user_id=0)) + response = client.get(url_for('main.edit_user_permissions', service_id=55555, user_id=0)) assert response.status_code == 200 @@ -47,7 +47,7 @@ def test_redirect_after_saving_user( with app_.test_client() as client: client.login(api_user_active) response = client.post(url_for( - 'main.edit_user', service_id=55555, user_id=0 + 'main.edit_user_permissions', service_id=55555, user_id=0 )) assert response.status_code == 302 @@ -104,3 +104,18 @@ def test_invite_user( assert page.h1.string.strip() == 'Manage team' flash_banner = page.find('div', class_='banner-default-with-tick').string.strip() assert flash_banner == 'Invite sent to test@example.gov.uk' + + +def test_cancel_invited_user_cancels_user_invitations(app_, api_user_active, mock_login, mocker): + with app_.test_request_context(): + with app_.test_client() as client: + mocker.patch('app.invite_api_client.cancel_invited_user') + import uuid + invited_user_id = uuid.uuid4() + client.login(api_user_active) + service_id = uuid.uuid4() + response = client.get(url_for('main.cancel_invited_user', service_id=service_id, + invited_user_id=invited_user_id)) + + assert response.status_code == 302 + assert response.location == url_for('main.manage_users', service_id=service_id, _external=True) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 1c132bf5c..e1e4762e9 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -149,6 +149,30 @@ def test_send_test_message_to_self( mock_s3_upload.assert_called_with(ANY, '12345', expected_data, 'eu-west-1') +def test_send_test_message_to_self( + app_, + mocker, + api_user_active, + mock_login, + mock_get_service, + mock_get_service_email_template, + mock_s3_upload +): + + expected_data = {'data': ['email address', 'test@user.gov.uk'], 'file_name': 'Test run'} + mocker.patch('app.main.views.send.s3download', return_value='email address\r\ntest@user.gov.uk') + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get( + url_for('main.send_message_to_self', service_id=12345, template_id=54321), + follow_redirects=True + ) + assert response.status_code == 200 + mock_s3_upload.assert_called_with(ANY, '12345', expected_data, 'eu-west-1') + + def test_download_example_csv( app_, mocker, diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index 58ce2bbed..4ba3beb09 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -252,7 +252,6 @@ def test_should_redirect_after_mobile_number_confirm(app_, response = client.post( url_for('main.user_profile_mobile_number_confirm'), data=data) - print(response.get_data(as_text=True)) assert response.status_code == 302 assert response.location == url_for( 'main.user_profile', _external=True) @@ -287,7 +286,6 @@ def test_should_redirect_after_password_change(app_, url_for('main.user_profile_password'), data=data) - print(response.get_data(as_text=True)) assert response.status_code == 302 assert response.location == url_for( 'main.user_profile', _external=True) diff --git a/tests/conftest.py b/tests/conftest.py index a3d4f54aa..24471d539 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -141,6 +141,17 @@ def mock_get_service_template(mocker): 'app.notifications_api_client.get_service_template', side_effect=_create) +@pytest.fixture(scope='function') +def mock_get_service_email_template(mocker): + def _create(service_id, template_id): + template = template_json( + template_id, "Two week reminder", "email", "Your vehicle tax is about to expire", service_id) + return {'data': template} + + return mocker.patch( + 'app.notifications_api_client.get_service_template', side_effect=_create) + + @pytest.fixture(scope='function') def mock_create_service_template(mocker): def _create(name, type_, content, service): @@ -570,7 +581,7 @@ def sample_invite(mocker, service_one): email_address = 'invited_user@test.gov.uk' service_id = service_one['id'] permissions = 'send_messages,manage_service,manage_api_keys' - created_at = datetime.datetime.now() + created_at = str(datetime.datetime.now()) return invite_json(id, from_user, service_id, email_address, permissions, created_at)