darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-09-06 15:38:25 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3176 from alphagov/require-uuids-in-urls

Browse Source 
Be stricter about the format of URL parameters
This commit is contained in:
Chris Hill-Scott
2019-11-13 10:33:31 +00:00
committed by GitHub
parent 059fb66fae bf981c308a
commit 24dc438446
33 changed files with 326 additions and 267 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
tests/app/main/views/test_find_users.py
View File

@@ -1,3 +1,5 @@
import uuid
import pytest
from bs4 import BeautifulSoup
from flask import url_for
@@ -91,23 +93,26 @@ def test_find_users_by_email_validates_against_empty_search_submission(
def test_user_information_page_shows_information_about_user(
client,
platform_admin_user,
mocker
mocker,
fake_uuid,
):
user_service_one = uuid.uuid4()
user_service_two = uuid.uuid4()
mocker.patch('app.user_api_client.get_user', side_effect=[
platform_admin_user,
user_json(name="Apple Bloom", services=[1, 2])
user_json(name="Apple Bloom", services=[user_service_one, user_service_two])
], autospec=True)
mocker.patch(
'app.user_api_client.get_organisations_and_services_for_user',
return_value={'organisations': [], 'services': [
{"id": 1, "name": "Fresh Orchard Juice", "restricted": True},
{"id": 2, "name": "Nature Therapy", "restricted": False},
{"id": user_service_one, "name": "Fresh Orchard Juice", "restricted": True},
{"id": user_service_two, "name": "Nature Therapy", "restricted": False},
]},
autospec=True
)
client.login(platform_admin_user)
response = client.get(url_for('main.user_information', user_id=345))
response = client.get(url_for('main.user_information', user_id=fake_uuid))
assert response.status_code == 200
document = html.fromstring(response.get_data(as_text=True))
@@ -129,7 +134,8 @@ def test_user_information_page_shows_information_about_user(
def test_user_information_page_displays_if_there_are_failed_login_attempts(
client,
platform_admin_user,
mocker
mocker,
fake_uuid,
):
mocker.patch('app.user_api_client.get_user', side_effect=[
platform_admin_user,
@@ -138,14 +144,11 @@ def test_user_information_page_displays_if_there_are_failed_login_attempts(
mocker.patch(
'app.user_api_client.get_organisations_and_services_for_user',
return_value={'organisations': [], 'services': [
{"id": 1, "name": "Fresh Orchard Juice", "restricted": True},
{"id": 2, "name": "Nature Therapy", "restricted": True},
]},
return_value={'organisations': [], 'services': []},
autospec=True
)
client.login(platform_admin_user)
response = client.get(url_for('main.user_information', user_id=345))
response = client.get(url_for('main.user_information', user_id=fake_uuid))
assert response.status_code == 200
document = html.fromstring(response.get_data(as_text=True))

2
tests/app/main/views/test_jobs.py
View File

@@ -493,7 +493,7 @@ def test_should_cancel_letter_job(
mocker,
active_user_with_permissions
):
job_id = uuid.uuid4()
job_id = str(uuid.uuid4())
job = job_json(
SERVICE_ONE_ID,
active_user_with_permissions,

14
tests/app/main/views/test_letter_branding.py
View File

@@ -51,9 +51,10 @@ def test_letter_branding_page_shows_full_branding_list(
def test_update_letter_branding_shows_the_current_letter_brand(
platform_admin_client,
mock_get_letter_branding_by_id,
fake_uuid,
):
response = platform_admin_client.get(
url_for('.update_letter_branding', branding_id='abc')
url_for('.update_letter_branding', branding_id=fake_uuid)
)
assert response.status_code == 200
@@ -81,7 +82,7 @@ def test_update_letter_branding_with_new_valid_file(
mock_delete_temp_files = mocker.patch('app.main.views.letter_branding.delete_letter_temp_file')
response = platform_admin_client.post(
url_for('.update_letter_branding', branding_id='abc'),
url_for('.update_letter_branding', branding_id=fake_uuid),
data={'file': (BytesIO(''.encode('utf-8')), filename)},
follow_redirects=True
)
@@ -97,10 +98,11 @@ def test_update_letter_branding_with_new_valid_file(
def test_update_letter_branding_when_uploading_invalid_file(
platform_admin_client,
mock_get_letter_branding_by_id
mock_get_letter_branding_by_id,
fake_uuid,
):
response = platform_admin_client.post(
url_for('.update_letter_branding', branding_id='abc'),
url_for('.update_letter_branding', branding_id=fake_uuid),
data={'file': (BytesIO(''.encode('utf-8')), 'test.png')},
follow_redirects=True
)
@@ -127,7 +129,7 @@ def test_update_letter_branding_deletes_any_temp_files_when_uploading_a_file(
mock_delete_temp_files = mocker.patch('app.main.views.letter_branding.delete_letter_temp_file')
response = platform_admin_client.post(
url_for('.update_letter_branding', branding_id='abc', logo=temp_logo),
url_for('.update_letter_branding', branding_id=fake_uuid, logo=temp_logo),
data={'file': (BytesIO(''.encode('utf-8')), 'new_uploaded_file.svg')},
follow_redirects=True
)
@@ -223,7 +225,7 @@ def test_update_letter_branding_shows_database_errors_on_name_field(
))
response = platform_admin_client.post(
url_for('.update_letter_branding', branding_id='abc'),
url_for('.update_letter_branding', branding_id=fake_uuid),
data={
'name': 'my brand',
'operation': 'branding-details'

3
tests/app/main/views/test_providers.py
View File

@@ -260,11 +260,12 @@ def test_should_show_edit_provider_form(
client,
platform_admin_user,
mocker,
fake_uuid,
):
mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider))
client.login(platform_admin_user, mocker)
response = client.get(url_for('main.edit_provider', provider_id='12345'))
response = client.get(url_for('main.edit_provider', provider_id=fake_uuid))
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')

2
tests/app/main/views/test_send.py
View File

@@ -34,7 +34,6 @@ from tests.conftest import (
SERVICE_ONE_ID,
active_caseworking_user,
active_user_with_permissions,
fake_uuid,
mock_get_international_service,
mock_get_live_service,
mock_get_service,
@@ -358,6 +357,7 @@ def test_shows_error_if_parsing_exception(
mock_get_service_template,
exception,
expected_error_message,
fake_uuid,
):
def _raise_exception_or_partial_exception(file_content, filename):

15
tests/app/main/views/test_service_settings.py
View File

@@ -2786,7 +2786,7 @@ def test_inbound_sms_sender_is_not_deleteable(
page = client_request.get(
'.service_edit_sms_sender',
service_id=SERVICE_ONE_ID,
sms_sender_id='1234',
sms_sender_id=fake_uuid,
)
back_link = page.select_one('.govuk-back-link')
@@ -2810,19 +2810,19 @@ def test_delete_sms_sender(
client_request.post(
'.service_delete_sms_sender',
service_id=SERVICE_ONE_ID,
sms_sender_id='1234',
sms_sender_id=fake_uuid,
_expected_redirect=url_for(
'main.service_sms_senders',
service_id=SERVICE_ONE_ID,
_external=True,
)
)
mock_delete.assert_called_once_with(service_id=SERVICE_ONE_ID, sms_sender_id='1234')
mock_delete.assert_called_once_with(service_id=SERVICE_ONE_ID, sms_sender_id=fake_uuid)
@pytest.mark.parametrize('fixture, hide_textbox, fixture_sender_id', [
(get_inbound_number_sms_sender, True, '1234'),
(get_default_sms_sender, False, '1234'),
@pytest.mark.parametrize('fixture, hide_textbox', [
(get_inbound_number_sms_sender, True),
(get_default_sms_sender, False),
])
def test_inbound_sms_sender_is_not_editable(
client_request,
@@ -2830,7 +2830,6 @@ def test_inbound_sms_sender_is_not_editable(
fake_uuid,
fixture,
hide_textbox,
fixture_sender_id,
mocker
):
fixture(mocker)
@@ -2838,7 +2837,7 @@ def test_inbound_sms_sender_is_not_editable(
page = client_request.get(
'.service_edit_sms_sender',
service_id=SERVICE_ONE_ID,
sms_sender_id=fixture_sender_id,
sms_sender_id=fake_uuid,
)
assert bool(page.find('input', attrs={'name': "sms_sender"})) != hide_textbox

3
tests/app/main/views/test_templates.py
View File

@@ -1260,6 +1260,7 @@ def test_should_not_allow_creation_of_template_through_form_without_correct_perm
endpoint,
data,
expected_error,
fake_uuid,
):
service_one['permissions'] = []
page = client_request.post(
@@ -1273,7 +1274,6 @@ def test_should_not_allow_creation_of_template_through_form_without_correct_perm
assert page.select(".govuk-back-link")[0]['href'] == url_for(
'.choose_template',
service_id=SERVICE_ONE_ID,
template_id='0',
)
@@ -1299,7 +1299,6 @@ def test_should_not_allow_creation_of_a_template_without_correct_permission(
assert page.select(".govuk-back-link")[0]['href'] == url_for(
'.choose_template',
service_id=service_one['id'],
template_id='0',
)

63
tests/app/main/views/test_uploads.py
View File

@@ -50,8 +50,14 @@ def test_get_upload_letter(client_request):
assert normalize_spaces(page.find('label', class_='file-upload-button').text) == 'Choose file'
def test_post_upload_letter_redirects_for_valid_file(mocker, active_user_with_permissions, service_one, client_request):
mocker.patch('uuid.uuid4', return_value='fake-uuid')
def test_post_upload_letter_redirects_for_valid_file(
mocker,
active_user_with_permissions,
service_one,
client_request,
fake_uuid,
):
mocker.patch('uuid.uuid4', return_value=fake_uuid)
antivirus_mock = mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True)
mocker.patch(
'app.main.views.uploads.sanitise_letter',
@@ -79,7 +85,7 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, active_user_with_pe
mock_s3.assert_called_once_with(
b'The sanitised content',
file_location='service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID),
file_location='service-{}/{}.pdf'.format(SERVICE_ONE_ID, fake_uuid),
status='valid',
page_count=1,
filename='tests/test_pdf_files/one_page_pdf.pdf',
@@ -90,7 +96,7 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, active_user_with_pe
assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf'
assert not page.find(id='validation-error-message')
assert page.find('input', {'type': 'hidden', 'name': 'file_id', 'value': 'fake-uuid'})
assert page.find('input', {'type': 'hidden', 'name': 'file_id', 'value': fake_uuid})
assert page.find('button', {'type': 'submit'}).text == 'Send 1 letter'
@@ -99,6 +105,7 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file(
active_user_with_permissions,
service_one,
client_request,
fake_uuid,
):
letter_template = {'template_type': 'letter',
'reply_to_text': '',
@@ -106,7 +113,7 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file(
'subject': 'hi',
'content': 'my letter'}
mocker.patch('uuid.uuid4', return_value='fake-uuid')
mocker.patch('uuid.uuid4', return_value=fake_uuid)
mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True)
mocker.patch(
'app.main.views.uploads.sanitise_letter',
@@ -145,7 +152,7 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file(
assert img['src'] == url_for(
'.view_letter_upload_as_preview',
service_id=SERVICE_ONE_ID,
file_id='fake-uuid',
file_id=fake_uuid,
page=page_no)
@@ -219,8 +226,8 @@ def test_post_choose_upload_file_when_file_is_malformed(mocker, client_request):
assert normalize_spaces(page.find('label', class_='file-upload-button').text) == 'Upload your file again'
def test_post_upload_letter_with_invalid_file(mocker, client_request):
mocker.patch('uuid.uuid4', return_value='fake-uuid')
def test_post_upload_letter_with_invalid_file(mocker, client_request, fake_uuid):
mocker.patch('uuid.uuid4', return_value=fake_uuid)
mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True)
mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3')
@@ -249,7 +256,7 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request):
mock_s3.assert_called_once_with(
file_contents,
file_location='service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID),
file_location='service-{}/{}.pdf'.format(SERVICE_ONE_ID, fake_uuid),
status='invalid',
page_count=1,
filename='tests/test_pdf_files/one_page_pdf.pdf',
@@ -261,14 +268,14 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request):
assert not page.find('button', {'type': 'submit'})
def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client_request):
def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client_request, fake_uuid):
letter_template = {'template_type': 'letter',
'reply_to_text': '',
'postage': 'first',
'subject': 'hi',
'content': 'my letter'}
mocker.patch('uuid.uuid4', return_value='fake-uuid')
mocker.patch('uuid.uuid4', return_value=fake_uuid)
mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True)
mocker.patch('app.main.views.uploads.upload_letter_to_s3')
mock_sanitise_response = Mock()
@@ -296,13 +303,17 @@ def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client
assert letter_images[0]['src'] == url_for(
'.view_letter_upload_as_preview',
service_id=SERVICE_ONE_ID,
file_id='fake-uuid',
file_id=fake_uuid,
page=1
)
def test_post_upload_letter_does_not_upload_to_s3_if_template_preview_raises_unknown_error(mocker, client_request):
mocker.patch('uuid.uuid4', return_value='fake-uuid')
def test_post_upload_letter_does_not_upload_to_s3_if_template_preview_raises_unknown_error(
mocker,
client_request,
fake_uuid,
):
mocker.patch('uuid.uuid4', return_value=fake_uuid)
mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True)
mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3')
@@ -320,7 +331,13 @@ def test_post_upload_letter_does_not_upload_to_s3_if_template_preview_raises_unk
assert not mock_s3.called
def test_uploaded_letter_preview(mocker, active_user_with_permissions, service_one, client_request):
def test_uploaded_letter_preview(
mocker,
active_user_with_permissions,
service_one,
client_request,
fake_uuid,
):
mocker.patch('app.main.views.uploads.service_api_client')
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid'})
@@ -331,7 +348,7 @@ def test_uploaded_letter_preview(mocker, active_user_with_permissions, service_o
page = client_request.get(
'main.uploaded_letter_preview',
service_id=SERVICE_ONE_ID,
file_id='fake-uuid',
file_id=fake_uuid,
original_filename='my_letter.pdf',
page_count=1,
status='valid',
@@ -342,7 +359,11 @@ def test_uploaded_letter_preview(mocker, active_user_with_permissions, service_o
assert page.find('div', class_='letter-sent')
def test_uploaded_letter_preview_does_not_show_send_button_if_service_in_trial_mode(mocker, client_request):
def test_uploaded_letter_preview_does_not_show_send_button_if_service_in_trial_mode(
mocker,
client_request,
fake_uuid,
):
mocker.patch('app.main.views.uploads.service_api_client')
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid'})
@@ -351,7 +372,7 @@ def test_uploaded_letter_preview_does_not_show_send_button_if_service_in_trial_m
page = client_request.get(
'main.uploaded_letter_preview',
service_id=SERVICE_ONE_ID,
file_id='fake-uuid',
file_id=fake_uuid,
original_filename='my_letter.pdf',
page_count=1,
status='valid',
@@ -368,6 +389,7 @@ def test_uploaded_letter_preview_image_shows_overlay_when_content_outside_printa
mocker,
logged_in_client,
mock_get_service,
fake_uuid,
):
mocker.patch(
'app.main.views.uploads.get_letter_pdf_and_metadata',
@@ -378,7 +400,7 @@ def test_uploaded_letter_preview_image_shows_overlay_when_content_outside_printa
return_value=make_response('page.html', 200))
logged_in_client.get(
url_for('main.view_letter_upload_as_preview', file_id='fake-uuid', service_id=SERVICE_ONE_ID, page=1)
url_for('main.view_letter_upload_as_preview', file_id=fake_uuid, service_id=SERVICE_ONE_ID, page=1)
)
template_preview_mock.assert_called_once_with('pdf_file', '1')
@@ -396,6 +418,7 @@ def test_uploaded_letter_preview_image_does_not_show_overlay_if_no_content_outsi
logged_in_client,
mock_get_service,
metadata,
fake_uuid,
):
mocker.patch(
'app.main.views.uploads.get_letter_pdf_and_metadata',
@@ -406,7 +429,7 @@ def test_uploaded_letter_preview_image_does_not_show_overlay_if_no_content_outsi
return_value=make_response('page.html', 200))
logged_in_client.get(
url_for('main.view_letter_upload_as_preview', file_id='fake-uuid', service_id=SERVICE_ONE_ID, page=1)
url_for('main.view_letter_upload_as_preview', file_id=fake_uuid, service_id=SERVICE_ONE_ID, page=1)
)
template_preview_mock.assert_called_once_with('pdf_file', '1')