Merge remote-tracking branch 'origin/master' into add_send_permission

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)

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')
 

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():

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():

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/<token>")
 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)

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/<service_id>/users/<user_id>", 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/<service_id>/cancel-invited-user/<invited_user_id>", 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':

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')

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()

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']

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

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
+                }

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):

app/templates/views/check.html

@@ -49,7 +49,7 @@
   {% if upload_result.valid %}
     <form method="post" enctype="multipart/form-data">
       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
-      <input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.rows|count, '' if upload_result.rows|count == 1 else 's') }}" />
+      <input type="submit" class="button" value="{{ "Send {} message{}".format(upload_result.rows|count, '' if upload_result.rows|count == 1 else 's') }}" />
       <a href="{{url_for('.send_messages', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
     </form>
   {% else %}

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') %}
-      <a href="{{ url_for('.edit_user', service_id=service_id, user_id=item.id)}}">Change</a>
-    {% 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') %}
-        <a href="{{ url_for('.edit_user', service_id=service_id, user_id=item.id)}}">Change</a>
-      {% endcall %}
+      {% if item.status == 'pending' %}
+        {% call field(align='right') %}
+          <a href="{{ url_for('.cancel_invited_user', service_id=service_id, invited_user_id=item.id)}}">Cancel invitation</a>
+        {% endcall %}
+      {% else %}
+        {% call field() %}
+          {{ item.status }}
+        {% endcall %}
+      {% endif %}
     {% endcall %}
   {% endif %}
 

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
 <div class="grid-row">
   <div class="column-two-thirds">
     <h1 class="heading-large">Create an account</h1>
-
-    <p>If you've used GOV.UK Notify before, <a href="">sign in to your account</a>.</p>
-
-    <p>
-    <label class="form-label" for="name-f1">Full name</label>
-      <input class="form-control-2-3" id="name-f1" type="text">
-      </p>
-      <p>
-      <label class="form-label" for="mobile">Mobile phone number</label>
-      <input class="form-control-1-4" id="mobile" type="text">
-      </p>
-      <p>
-      <label class="form-label" for="password">Create a password</label>
-      <input class="form-control-1-4" id="password" type="password"><br>
-      <span class="font-xsmall">Your password must have at least 10 characters</span></label>
-      </p>
-
-      <p>
-      <a class="button" href="verify-mobile" role="button">Continue</a>
-      </p>
+    <form method="post" autocomplete="nope">
+      {{ 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}}
+    </form>
   </div>
 </div>
 

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'])

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)

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,

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)

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)