From 6ba13a65136511b98ffba5c705cb75bab02f5fd1 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Wed, 2 Mar 2016 15:25:04 +0000 Subject: [PATCH] [WIP] New user can now accept invite and will be made to register. On succesful register and verfication they will be added to service and forwarded to dashboard. Nothing is done yet with the permissions requested in the invite to the user. --- app/main/forms.py | 12 +- app/main/views/add_service.py | 22 ++- app/main/views/index.py | 6 - app/main/views/invites.py | 10 +- app/main/views/register.py | 77 +++++++--- app/main/views/verify.py | 1 + app/notify_client/models.py | 9 ++ app/notify_client/user_api_client.py | 10 +- app/templates/views/register-from-invite.html | 30 ++-- tests/app/main/views/test_accept_invite.py | 144 +++++++++++++++++- tests/app/main/views/test_user_profile.py | 2 - tests/conftest.py | 2 +- 12 files changed, 257 insertions(+), 68 deletions(-) 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/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/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/models.py b/app/notify_client/models.py index d5caf5b33..8c51666eb 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -125,3 +125,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/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_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 7870d9169..152621b2f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -572,7 +572,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)