Make search by recipient form POST not GET

Phone numbers and email addresses are showing up in URLs where we let
users search for sent notifications by phone number or email address.

`GET` requests put the form data as a query string in the URL. This is
problematic when people are searching by a recipient’s phone number or
email address, because the URL may show up:
- in our server logs
- in our analytics
- in the user’s browser history

This is bad because these are all places where we don’t want
people’s personal information. It’s not too bad when this is happening
a handful of times. But it would be bad if we kept aggregating this
information because it would allow us to track users across services.

So, while it’s not especially RESTful, it’s better for the search form
to submit as a `POST` request. This way the phone number or email
address goes in the body of the request and does not show up in the URL.
This commit is contained in:
Chris Hill-Scott
2017-06-13 11:05:57 +01:00
parent b7b4d38330
commit e65dcbe199
3 changed files with 48 additions and 19 deletions

View File

@@ -190,7 +190,7 @@ def view_job_updates(service_id, job_id):
))
@main.route('/services/<service_id>/notifications/<message_type>')
@main.route('/services/<service_id>/notifications/<message_type>', methods=['GET', 'POST'])
@login_required
@user_has_permissions('view_activity', admin_override=True)
def view_notifications(service_id, message_type):
@@ -200,8 +200,8 @@ def view_notifications(service_id, message_type):
message_type=message_type,
status=request.args.get('status') or 'sending,delivered,failed',
page=request.args.get('page', 1),
to=request.args.get('to'),
search_form=SearchNotificationsForm(to=request.args.get('to')),
to=request.form.get('to', ''),
search_form=SearchNotificationsForm(to=request.form.get('to', '')),
)
@@ -245,7 +245,7 @@ def get_notifications(service_id, message_type, status_override=None):
template_type=[message_type],
status=filter_args.get('status'),
limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'],
to=request.args.get('to'),
to=request.form.get('to', ''),
)
url_args = {

View File

@@ -21,7 +21,7 @@
) }}
<form
method="get"
method="post"
action="{{ url_for('.view_notifications', service_id=current_service.id, message_type=message_type) }}"
class="grid-row"
>
@@ -34,6 +34,7 @@
) }}
</div>
<div class="column-one-quarter align-button-with-textbox">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button" value="Search">
</div>
</form>

View File

@@ -70,14 +70,27 @@ def test_can_show_notifications(
to_argument,
expected_to_argument,
):
response = logged_in_client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
message_type=message_type,
status=status_argument,
page=page_argument,
to=to_argument,
))
if expected_to_argument:
response = logged_in_client.post(
url_for(
'main.view_notifications',
service_id=service_one['id'],
message_type=message_type,
status=status_argument,
page=page_argument,
),
data={
'to': to_argument
}
)
else:
response = logged_in_client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
message_type=message_type,
status=status_argument,
page=page_argument,
))
assert response.status_code == 200
content = response.get_data(as_text=True)
notifications = notification_json(service_one['id'])
@@ -119,17 +132,25 @@ def test_can_show_notifications(
assert json_content.keys() == {'counts', 'notifications'}
@pytest.mark.parametrize("initial_query_arguments, expected_status_field_value, expected_search_box_contents", [
@pytest.mark.parametrize((
'initial_query_arguments,'
'form_post_data,'
'expected_status_field_value,'
'expected_search_box_contents'
), [
(
{
'message_type': 'sms',
},
{},
'sending,delivered,failed',
'',
),
(
{
'message_type': 'sms',
},
{
'to': '+33(0)5-12-34-56-78',
},
'sending,delivered,failed',
@@ -140,6 +161,8 @@ def test_can_show_notifications(
'status': 'failed',
'message_type': 'email',
'page': '99',
},
{
'to': 'test@example.com',
},
'failed',
@@ -151,17 +174,22 @@ def test_search_recipient_form(
mock_get_notifications,
mock_get_detailed_service,
initial_query_arguments,
form_post_data,
expected_status_field_value,
expected_search_box_contents,
):
response = logged_in_client.get(url_for(
'main.view_notifications',
service_id=SERVICE_ONE_ID,
**initial_query_arguments
))
response = logged_in_client.post(
url_for(
'main.view_notifications',
service_id=SERVICE_ONE_ID,
**initial_query_arguments
),
data=form_post_data
)
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.find("form")['method'] == 'post'
action_url = page.find("form")['action']
url = urlparse(action_url)
assert url.path == '/services/{}/notifications/{}'.format(