darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-09-11 10:28:41 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #376 from alphagov/dont-accepted-csv-with-restricted-recipients

Browse Source 
Show error if restricted service uploads real CSV
This commit is contained in:
Chris Hill-Scott
2016-04-05 10:16:20 +01:00
parent 6ad3c8a6ed 7a0a81c07c
commit b912fa3f66
4 changed files with 50 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/main/views/send.py
+9 -3
View File
@@ -2,6 +2,7 @@ import csv
import io import io
import json import json
import uuid import uuid
import itertools
from contextlib import suppress from contextlib import suppress
from flask import ( from flask import (
@@ -25,8 +26,8 @@ from app.main.uploader import (
s3upload, s3upload,
s3download s3download
) )
from app import (job_api_client, service_api_client, current_service) from app import job_api_client, service_api_client, current_service, user_api_client
from app.utils import (user_has_permissions, get_errors_for_csv) from app.utils import user_has_permissions, get_errors_for_csv
def get_send_button_text(template_type, number_of_messages): def get_send_button_text(template_type, number_of_messages):
@@ -204,6 +205,8 @@ def check_messages(service_id, template_type, upload_id):
if not session.get('upload_data'): if not session.get('upload_data'):
return redirect(url_for('main.choose_template', service_id=service_id, template_type=template_type)) return redirect(url_for('main.choose_template', service_id=service_id, template_type=template_type))
users = user_api_client.get_users_for_service(service_id=service_id)
contents = s3download(service_id, upload_id) contents = s3download(service_id, upload_id)
if not contents: if not contents:
flash('There was a problem reading your upload file') flash('There was a problem reading your upload file')
@@ -223,7 +226,10 @@ def check_messages(service_id, template_type, upload_id):
template_type=template.template_type, template_type=template.template_type,
placeholders=template.placeholders, placeholders=template.placeholders,
max_initial_rows_shown=15, max_initial_rows_shown=15,
max_errors_shown=15 max_errors_shown=15,
whitelist=itertools.chain.from_iterable(
[user.mobile_number, user.email_address] for user in users
) if current_service['restricted'] else None
) )
with suppress(StopIteration): with suppress(StopIteration):
requirements.txt
+1 -1
View File
@@ -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-python-client.git@0.3.1#egg=notifications-python-client==0.3.1
git+https://github.com/alphagov/notifications-utils.git@3.1.3#egg=notifications-utils==3.1.3 git+https://github.com/alphagov/notifications-utils.git@3.2.0#egg=notifications-utils==3.2.0
tests/app/main/views/test_send.py
+33 -28
View File
@@ -16,7 +16,8 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
mock_get_service, mock_get_service,
mock_get_service_template, mock_get_service_template,
mock_s3_upload, mock_s3_upload,
mock_has_permissions mock_has_permissions,
mock_get_users_by_service
): ):
contents = u'phone number,name\n+44 123,test1\n+44 456,test2' contents = u'phone number,name\n+44 123,test1\n+44 456,test2'
@@ -55,10 +56,11 @@ def test_send_test_sms_message_to_self(
mock_get_service, mock_get_service,
mock_get_service_template, mock_get_service_template,
mock_s3_upload, mock_s3_upload,
mock_has_permissions mock_has_permissions,
mock_get_users_by_service
): ):
expected_data = {'data': 'phone number\r\n+4412341234\r\n', 'file_name': 'Test run'} expected_data = {'data': 'phone number\r\n07700 900762\r\n', 'file_name': 'Test run'}
mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234') mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
with app_.test_request_context(): with app_.test_request_context():
@@ -80,7 +82,8 @@ def test_send_test_email_message_to_self(
mock_get_service, mock_get_service,
mock_get_service_email_template, mock_get_service_email_template,
mock_s3_upload, mock_s3_upload,
mock_has_permissions mock_has_permissions,
mock_get_users_by_service
): ):
expected_data = {'data': 'email address\r\ntest@user.gov.uk\r\n', 'file_name': 'Test run'} expected_data = {'data': 'email address\r\ntest@user.gov.uk\r\n', 'file_name': 'Test run'}
@@ -136,7 +139,7 @@ def test_download_example_csv(
follow_redirects=True follow_redirects=True
) )
assert response.status_code == 200 assert response.status_code == 200
assert response.get_data(as_text=True) == 'phone number\r\n+4412341234\r\n+4412341234\r\n' assert response.get_data(as_text=True) == 'phone number\r\n07700 900762\r\n07700 900762\r\n'
assert 'text/csv' in response.headers['Content-Type'] assert 'text/csv' in response.headers['Content-Type']
@@ -148,31 +151,32 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(
mock_get_service, mock_get_service,
mock_get_service_template, mock_get_service_template,
mock_s3_upload, mock_s3_upload,
mock_has_permissions mock_has_permissions,
mock_get_users_by_service
): ):
mocker.patch( mocker.patch(
'app.main.views.send.s3download', 'app.main.views.send.s3download',
return_value=""" return_value="""
phone number phone number
+44 7700 9009 01 07700 900701
+44 7700 9009 02 07700 900702
+44 7700 9009 03 07700 900703
+44 7700 9009 04 07700 900704
+44 7700 9009 05 07700 900705
+44 7700 9009 06 07700 900706
+44 7700 9009 07 07700 900707
+44 7700 9009 08 07700 900708
+44 7700 9009 09 07700 900709
+44 7700 9009 10 07700 900710
+44 7700 9009 11 07700 900711
+44 7700 9009 12 07700 900712
+44 7700 9009 13 07700 900713
+44 7700 9009 14 07700 900714
+44 7700 9009 15 07700 900715
+44 7700 9009 99 07700 900799
+44 7700 9009 99 07700 900799
+44 7700 9009 99 07700 900799
""" """
) )
@@ -192,9 +196,9 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(
content = response.get_data(as_text=True) content = response.get_data(as_text=True)
assert response.status_code == 200 assert response.status_code == 200
assert '+44 7700 9009 01' in content assert '07700 900701' in content
assert '+44 7700 9009 15' in content assert '07700 900715' in content
assert '+44 7700 9009 16' not in content assert '07700 900716' not in content
assert '3 rows not shown' in content assert '3 rows not shown' in content
@@ -245,7 +249,8 @@ def test_check_messages_should_revalidate_file_when_uploading_file(
mock_get_service_template, mock_get_service_template,
mock_s3_upload, mock_s3_upload,
mocker, mocker,
mock_has_permissions mock_has_permissions,
mock_get_users_by_service
): ):
service_id = service_one['id'] service_id = service_one['id']
tests/conftest.py
+7 -7
View File
@@ -245,7 +245,7 @@ def api_user_pending():
'name': 'Test User', 'name': 'Test User',
'password': 'somepassword', 'password': 'somepassword',
'email_address': 'test@user.gov.uk', 'email_address': 'test@user.gov.uk',
'mobile_number': '+4412341234', 'mobile_number': '07700 900762',
'state': 'pending', 'state': 'pending',
'failed_login_count': 0, 'failed_login_count': 0,
'permissions': {} 'permissions': {}
@@ -261,7 +261,7 @@ def platform_admin_user():
'name': 'Platform admin user', 'name': 'Platform admin user',
'password': 'somepassword', 'password': 'somepassword',
'email_address': 'platform@admin.gov.uk', 'email_address': 'platform@admin.gov.uk',
'mobile_number': '+4472341234', 'mobile_number': '07700 900762',
'state': 'active', 'state': 'active',
'failed_login_count': 0, 'failed_login_count': 0,
'permissions': {}, 'permissions': {},
@@ -278,7 +278,7 @@ def api_user_active():
'name': 'Test User', 'name': 'Test User',
'password': 'somepassword', 'password': 'somepassword',
'email_address': 'test@user.gov.uk', 'email_address': 'test@user.gov.uk',
'mobile_number': '+4412341234', 'mobile_number': '07700 900762',
'state': 'active', 'state': 'active',
'failed_login_count': 0, 'failed_login_count': 0,
'permissions': {}, 'permissions': {},
@@ -296,7 +296,7 @@ def active_user_with_permissions():
'name': 'Test User', 'name': 'Test User',
'password': 'somepassword', 'password': 'somepassword',
'email_address': 'test@user.gov.uk', 'email_address': 'test@user.gov.uk',
'mobile_number': '+4412341234', 'mobile_number': '07700 900762',
'state': 'active', 'state': 'active',
'failed_login_count': 0, 'failed_login_count': 0,
'permissions': {SERVICE_ONE_ID: ['send_texts', 'permissions': {SERVICE_ONE_ID: ['send_texts',
@@ -320,7 +320,7 @@ def api_user_locked():
'name': 'Test User', 'name': 'Test User',
'password': 'somepassword', 'password': 'somepassword',
'email_address': 'test@user.gov.uk', 'email_address': 'test@user.gov.uk',
'mobile_number': '+4412341234', 'mobile_number': '07700 900762',
'state': 'active', 'state': 'active',
'failed_login_count': 5, 'failed_login_count': 5,
'permissions': {} 'permissions': {}
@@ -336,7 +336,7 @@ def api_user_request_password_reset():
'name': 'Test User', 'name': 'Test User',
'password': 'somepassword', 'password': 'somepassword',
'email_address': 'test@user.gov.uk', 'email_address': 'test@user.gov.uk',
'mobile_number': '+4412341234', 'mobile_number': '07700 900762',
'state': 'active', 'state': 'active',
'failed_login_count': 5, 'failed_login_count': 5,
'permissions': {}, 'permissions': {},
@@ -353,7 +353,7 @@ def api_user_changed_password():
'name': 'Test User', 'name': 'Test User',
'password': 'somepassword', 'password': 'somepassword',
'email_address': 'test@user.gov.uk', 'email_address': 'test@user.gov.uk',
'mobile_number': '+4412341234', 'mobile_number': '07700 900762',
'state': 'active', 'state': 'active',
'failed_login_count': 5, 'failed_login_count': 5,
'permissions': {}, 'permissions': {},