Merge branch 'master' into fix-permissions

app/main/views/choose_service.py

@@ -1,13 +1,29 @@
 from flask import (render_template, redirect, url_for, session)
 from flask_login import login_required, current_user
-from app.main.dao import services_dao
+from app.main.dao.services_dao import ServicesBrowsableItem
+from app import service_api_client
 from app.main import main
 
 
 @main.route("/services")
 @login_required
 def choose_service():
-    services = services_dao.get_services(current_user.id)
     return render_template(
         'views/choose-service.html',
-        services=[services_dao.ServicesBrowsableItem(x) for x in services['data']])
+        services=[ServicesBrowsableItem(x) for x in service_api_client.get_services()['data']]
+    )
+
+
+@main.route("/services-or-dashboard")
+def show_all_services_or_dashboard():
+
+    if current_user.is_authenticated():
+
+        services = service_api_client.get_services()['data']
+
+        if 1 == len(services):
+            return redirect(url_for('.service_dashboard', service_id=services[0]['id']))
+        else:
+            return redirect(url_for('.choose_service'))
+
+    return redirect(url_for('main.index'))

app/main/views/send.py

@@ -26,8 +26,10 @@ from app.main.uploader import (
     s3download
 )
 from app.main.dao import templates_dao
-from app.main.dao import services_dao
+from app import (
-from app import job_api_client
+    job_api_client,
+    service_api_client
+)
 from app.utils import user_has_permissions, get_errors_for_csv
 
 
@@ -74,7 +76,7 @@ def get_page_headings(template_type):
                       admin_override=True, or_=True)
 def choose_template(service_id, template_type):
 
-    service = services_dao.get_service_by_id_or_404(service_id)
+    service = service_api_client.get_service(service_id)['data']
 
     if template_type not in ['email', 'sms']:
         abort(404)
@@ -102,6 +104,12 @@ def choose_template(service_id, template_type):
 @user_has_permissions('send_texts', 'send_emails', 'send_letters')
 def send_messages(service_id, template_id):
 
+    service = service_api_client.get_service(service_id)['data']
+    template = Template(
+        templates_dao.get_service_template_or_404(service_id, template_id)['data'],
+        prefix=service['name']
+    )
+
     form = CsvUploadForm()
     if form.validate_on_submit():
         try:
@@ -121,18 +129,13 @@ def send_messages(service_id, template_id):
             }
             return redirect(url_for('.check_messages',
                                     service_id=service_id,
-                                    upload_id=upload_id))
+                                    upload_id=upload_id,
+                                    template_type=template.template_type))
         except ValueError as e:
             flash('There was a problem uploading: {}'.format(form.file.data.filename))
             flash(str(e))
             return redirect(url_for('.send_messages', service_id=service_id, template_id=template_id))
 
-    service = services_dao.get_service_by_id_or_404(service_id)
-    template = Template(
-        templates_dao.get_service_template_or_404(service_id, template_id)['data'],
-        prefix=service['name']
-    )
-
     return render_template(
         'views/send.html',
         template=template,
@@ -188,12 +191,14 @@ def send_message_to_self(service_id, template_id):
         'data': output.getvalue()
     }
     upload_id = str(uuid.uuid4())
+
     s3upload(upload_id, service_id, filedata, current_app.config['AWS_REGION'])
     session['upload_data'] = {"template_id": template_id, "original_file_name": filedata['file_name']}
 
     return redirect(url_for('.check_messages',
                             service_id=service_id,
-                            upload_id=upload_id))
+                            upload_id=upload_id,
+                            template_type=template.template_type))
 
 
 @main.route("/services/<service_id>/send/<template_id>/from-api", methods=['GET'])
@@ -218,12 +223,15 @@ def send_from_api(service_id, template_id):
     )
 
 
-@main.route("/services/<service_id>/check/<upload_id>", methods=['GET'])
+@main.route("/services/<service_id>/<template_type>/check/<upload_id>", methods=['GET'])
 @login_required
 @user_has_permissions('send_texts', 'send_emails', 'send_letters')
-def check_messages(service_id, upload_id):
+def check_messages(service_id, template_type, upload_id):
 
-    service = services_dao.get_service_by_id_or_404(service_id)
+    if not session.get('upload_data'):
+        return redirect(url_for('main.choose_template', service_id=service_id, template_type=template_type))
+
+    service = service_api_client.get_service(service_id)['data']
 
     contents = s3download(service_id, upload_id)
     if not contents:
@@ -270,17 +278,18 @@ def check_messages(service_id, upload_id):
         send_button_text=get_send_button_text(template.template_type, session['upload_data']['notification_count']),
         service_id=service_id,
         service=service,
+        upload_id=upload_id,
         form=CsvUploadForm()
     )
 
 
-@main.route("/services/<service_id>/check/<upload_id>", methods=['POST'])
+@main.route("/services/<service_id>/start-job/<upload_id>", methods=['POST'])
 @login_required
 @user_has_permissions('send_texts', 'send_emails', 'send_letters')
 def start_job(service_id, upload_id):
 
     upload_data = session['upload_data']
-    services_dao.get_service_by_id_or_404(service_id)
+    service = service_api_client.get_service(service_id)['data']
 
     if request.files or not upload_data.get('valid'):
         # The csv was invalid, validate the csv again

app/main/views/sign_in.py

@@ -31,6 +31,9 @@ def sign_in():
     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:
             # Remember me login
             if not login_fresh() and \
@@ -45,9 +48,7 @@ def sign_in():
                     return redirect(url_for('main.choose_service'))
 
             session['user_details'] = {"email": user.email_address, "id": user.id}
-            if user.state == 'pending':
+            if user.is_active():
-                return redirect(url_for('.verify'))
-            elif user.is_active():
                 user_api_client.send_verify_code(user.id, 'sms', user.mobile_number)
                 if request.args.get('next'):
                     return redirect(url_for('.two_factor', next=request.args.get('next')))

app/main/views/verify.py

@@ -6,7 +6,8 @@ from flask import (
     session,
     url_for,
     current_app,
-    flash
+    flash,
+    abort
 )
 
 from itsdangerous import SignatureExpired
@@ -55,10 +56,17 @@ def verify_email(token):
 
         token_data = json.loads(token_data)
         verified = user_api_client.check_verify_code(token_data['user_id'], token_data['secret_code'], 'email')
+        user = user_api_client.get_user(token_data['user_id'])
+        if not user:
+            abort(404)
+
+        if user.is_active():
+            flash("That verification link has expired.")
+            return redirect(url_for('main.sign_in'))
+
+        session['user_details'] = {"email": user.email_address, "id": user.id}
         if verified[0]:
-            user = user_api_client.get_user(token_data['user_id'])
             user_api_client.send_verify_code(user.id, 'sms', user.mobile_number)
-            session['user_details'] = {"email": user.email_address, "id": user.id}
             return redirect('verify')
         else:
             if verified[1] == 'Code has expired':

app/templates/admin_template.html

@@ -59,12 +59,7 @@
 
 {% set global_header_text = "GOV.UK Notify" %}
 
-
+{% set homepage_url = url_for('main.show_all_services_or_dashboard') %}
-{% if not current_user.is_authenticated() %}
-    {% set homepage_url = url_for('main.index') %}
-{% else %}
-    {% set homepage_url = url_for('main.choose_service') %}
-{% endif %}
 
 {% block content %}
   <div id="content">

app/templates/views/check.html

@@ -64,7 +64,7 @@
   {% if errors %}
     {{file_upload(form.file, button_text='Re-upload your file')}}
   {% else %}
-    <form method="post" enctype="multipart/form-data">
+    <form method="post" enctype="multipart/form-data" action="{{url_for('main.start_job', service_id=service_id, upload_id=upload_id)}}">
       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
       <input type="submit" class="button" value="{{ send_button_text }}" />
       <a href="{{url_for('.send_messages', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>

requirements.txt

@@ -14,4 +14,4 @@ Pygments==2.0.2
 
 git+https://github.com/alphagov/notifications-python-client.git@0.3.1#egg=notifications-python-client==0.3.1
 
-git+https://github.com/alphagov/notifications-utils.git@3.1.1#egg=notifications-utils==3.1.1
+git+https://github.com/alphagov/notifications-utils.git@3.1.3#egg=notifications-utils==3.1.3

tests/app/main/views/test_choose_services.py

@@ -21,6 +21,47 @@ def test_should_show_choose_services_page(app_,
         assert 'List all services' not in resp_data
 
 
+def test_redirect_if_only_one_service(
+    app_,
+    mock_login,
+    mock_get_user,
+    api_user_active,
+    mock_get_services_with_one_service
+):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            client.login(api_user_active)
+            response = client.get(url_for('main.show_all_services_or_dashboard'))
+
+        service = mock_get_services_with_one_service.side_effect()['data'][0]
+        assert response.status_code == 302
+        assert response.location == url_for('main.service_dashboard', service_id=service['id'], _external=True)
+
+
+def test_redirect_if_multiple_services(
+    app_,
+    mock_login,
+    mock_get_user,
+    api_user_active,
+    mock_get_services
+):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            client.login(api_user_active)
+            response = client.get(url_for('main.show_all_services_or_dashboard'))
+
+        assert response.status_code == 302
+        assert response.location == url_for('main.choose_service', _external=True)
+
+
+def test_should_redirect_if_not_logged_in(app_):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            response = client.get(url_for('main.show_all_services_or_dashboard'))
+            assert response.status_code == 302
+            assert response.location == url_for('main.index', _external=True)
+
+
 def test_should_show_all_services_for_platform_admin_user(app_,
                                                           platform_admin_user,
                                                           mock_get_services,

tests/app/main/views/test_send.py

@@ -40,7 +40,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
         assert 'Re-upload your file' in content
 
 
-def test_send_test_message_to_self(
+def test_send_test_sms_message_to_self(
     app_,
     mocker,
     api_user_active,
@@ -51,7 +51,7 @@ def test_send_test_message_to_self(
     mock_has_permissions
 ):
 
-    expected_data = {'data': ['phone number', '+4412341234'], 'file_name': 'Test run'}
+    expected_data = {'data': 'phone number\r\n+4412341234\r\n', 'file_name': 'Test run'}
     mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
 
     with app_.test_request_context():
@@ -65,7 +65,7 @@ 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(
+def test_send_test_email_message_to_self(
     app_,
     mocker,
     api_user_active,
@@ -247,11 +247,8 @@ def test_check_messages_should_revalidate_file_when_uploading_file(
         'app.main.views.send.s3download',
         return_value="""
             phone number,name,,,
-            ++44 7700 900981,test1,,,
+            123,test1,,,
-            +44 7700 900981,test2,,,
+            123,test2,,,
-             ,,,
-              ,,, \t \t
-
         """
     )
     with app_.test_request_context():
@@ -263,7 +260,7 @@ def test_check_messages_should_revalidate_file_when_uploading_file(
                                           'notification_count': job_data['notification_count'],
                                           'valid': True}
             response = client.post(
-                url_for('main.check_messages', service_id=service_id, upload_id=job_data['id']),
+                url_for('main.start_job', service_id=service_id, upload_id=job_data['id']),
                 data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')},
                 content_type='multipart/form-data',
                 follow_redirects=True

tests/app/main/views/test_sign_in.py

@@ -1,9 +1,5 @@
-
-from datetime import datetime
-
-from app.main.dao import users_dao
-
 from flask import url_for
+from bs4 import BeautifulSoup
 
 
 def test_render_sign_in_returns_sign_in_template(app_):
@@ -75,9 +71,11 @@ def test_should_return_redirect_when_user_is_pending(app_,
         response = app_.test_client().post(
             url_for('main.sign_in'), data={
                 'email_address': 'pending_user@example.gov.uk',
-                'password': 'val1dPassw0rd!'})
+                'password': 'val1dPassw0rd!'}, follow_redirects=True)
-        assert response.status_code == 302
+        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-        assert response.location == url_for('main.verify', _external=True)
+        assert page.h1.string == 'Sign in'
+        flash_banner = page.find('div', class_='banner-dangerous').string.strip()
+        assert flash_banner == "You haven't verified your email or mobile number yet."
 
 
 def test_not_fresh_session_login(app_,

tests/app/main/views/test_verify.py

@@ -70,18 +70,18 @@ def test_should_return_200_when_sms_code_is_wrong(app_,
 
 def test_verify_email_redirects_to_verify_if_token_valid(app_,
                                                          mocker,
-                                                         api_user_active,
+                                                         api_user_pending,
-                                                         mock_get_user,
+                                                         mock_get_user_pending,
                                                          mock_send_verify_code,
                                                          mock_check_verify_code):
     import json
-    token_data = {"user_id": api_user_active.id, "secret_code": 12345}
+    token_data = {"user_id": api_user_pending.id, "secret_code": 12345}
     mocker.patch('utils.url_safe_token.check_token', return_value=json.dumps(token_data))
 
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
-                session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
+                session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id}
 
             response = client.get(url_for('main.verify_email', token='notreal'))
 
@@ -91,7 +91,7 @@ def test_verify_email_redirects_to_verify_if_token_valid(app_,
 
 def test_verify_email_redirects_to_email_sent_if_token_expired(app_,
                                                                mocker,
-                                                               api_user_active,
+                                                               api_user_pending,
                                                                mock_check_verify_code):
     from itsdangerous import SignatureExpired
     mocker.patch('utils.url_safe_token.check_token', side_effect=SignatureExpired('expired'))
@@ -99,7 +99,7 @@ def test_verify_email_redirects_to_email_sent_if_token_expired(app_,
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
-                session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
+                session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id}
 
             response = client.get(url_for('main.verify_email', token='notreal'))
 
@@ -109,8 +109,8 @@ def test_verify_email_redirects_to_email_sent_if_token_expired(app_,
 
 def test_verify_email_redirects_to_email_sent_if_token_used(app_,
                                                             mocker,
-                                                            api_user_active,
+                                                            api_user_pending,
-                                                            mock_get_user,
+                                                            mock_get_user_pending,
                                                             mock_send_verify_code,
                                                             mock_check_verify_code_code_expired):
     from itsdangerous import SignatureExpired
@@ -119,9 +119,31 @@ def test_verify_email_redirects_to_email_sent_if_token_used(app_,
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
-                session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
+                session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id}
 
             response = client.get(url_for('main.verify_email', token='notreal'))
 
             assert response.status_code == 302
             assert response.location == url_for('main.resend_email_verification', _external=True)
+
+
+def test_verify_email_redirects_to_sign_in_if_user_active(app_,
+                                                          mocker,
+                                                          api_user_active,
+                                                          mock_get_user,
+                                                          mock_send_verify_code,
+                                                          mock_check_verify_code):
+    import json
+    token_data = {"user_id": api_user_active.id, "secret_code": 12345}
+    mocker.patch('utils.url_safe_token.check_token', return_value=json.dumps(token_data))
+
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            with client.session_transaction() as session:
+                session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
+
+            response = client.get(url_for('main.verify_email', token='notreal'), follow_redirects=True)
+            page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+            assert page.h1.text == 'Sign in'
+            flash_banner = page.find('div', class_='banner-dangerous').string.strip()
+            assert flash_banner == "That verification link has expired."