Let users download a CSV of inbound messages

In user research, we’ve seen users copy/pasting the contents of the
inbound SMS page into a spreadsheet, in order to keep a record of the
messages they receive. They even went as far as to write a macro which
fixed the errors caused by copying and pasting.

It would be much easier if we just gave them the data already in a
spreadsheet format. Which is what this commit does.

One caveat is that, because spreadsheets can contain executable code (ie
formulas), and because we’re populating the spreadsheet with
user-submitted data (albeit via SMS) we need to be careful about
injection attacks.

The details of how these attacks work are detailed here (interesting
reading): http://georgemauer.net/2017/10/07/csv-injection.html

The mitigation is to not allow characters which initialise a formula
at the start of the cell.
This commit is contained in:
Chris Hill-Scott
2017-10-17 11:41:12 +01:00
parent ad8a35b045
commit c9b2211bd3
5 changed files with 126 additions and 2 deletions

View File

@@ -231,7 +231,7 @@ def format_datetime(date):
def format_datetime_24h(date): def format_datetime_24h(date):
return '{} at {}'.format( return '{} at {}'.format(
format_date(date), format_date(date),
gmt_timezones(date).strftime('%H:%M') format_time_24h(date),
) )
@@ -256,6 +256,21 @@ def format_datetime_relative(date):
) )
def format_datetime_numeric(date):
return '{} {}'.format(
format_date_numeric(date),
format_time_24h(date),
)
def format_date_numeric(date):
return gmt_timezones(date).strftime('%Y-%m-%d')
def format_time_24h(date):
return gmt_timezones(date).strftime('%H:%M')
def get_human_day(time): def get_human_day(time):
# Add 1 hour to get midnight today instead of midnight tomorrow # Add 1 hour to get midnight today instead of midnight tomorrow

View File

@@ -39,6 +39,11 @@
margin-top: $gutter * 4 / 3; margin-top: $gutter * 4 / 3;
} }
.top-gutter-2-3 {
@extend %top-gutter;
margin-top: $gutter-half;
}
%bottom-gutter, %bottom-gutter,
.bottom-gutter { .bottom-gutter {
@extend %contain-floats; @extend %contain-floats;

View File

@@ -6,7 +6,8 @@ from flask import (
session, session,
jsonify, jsonify,
request, request,
abort abort,
Response,
) )
from flask_login import login_required from flask_login import login_required
@@ -20,6 +21,9 @@ from app import (
service_api_client, service_api_client,
template_statistics_client, template_statistics_client,
inbound_number_client, inbound_number_client,
format_datetime_short,
format_date_numeric,
format_datetime_numeric,
) )
from app.statistics_utils import get_formatted_percentage, add_rate_to_job from app.statistics_utils import get_formatted_percentage, add_rate_to_job
from app.utils import ( from app.utils import (
@@ -27,6 +31,7 @@ from app.utils import (
get_current_financial_year, get_current_financial_year,
FAILURE_STATUSES, FAILURE_STATUSES,
REQUESTED_STATUSES, REQUESTED_STATUSES,
Spreadsheet,
) )
@@ -161,6 +166,31 @@ def inbox_updates(service_id):
return jsonify(get_inbox_partials(service_id)) return jsonify(get_inbox_partials(service_id))
@main.route("/services/<service_id>/inbox.csv")
@login_required
@user_has_permissions('view_activity', admin_override=True)
def inbox_download(service_id):
return Response(
Spreadsheet.from_rows(
[[
'Phone number',
'Message',
'Received',
]] + [[
message['user_number'],
message['content'].lstrip(('=+-@')),
format_datetime_numeric(message['created_at']),
] for message in service_api_client.get_inbound_sms(service_id)]
).as_csv_data,
mimetype='text/csv',
headers={
'Content-Disposition': 'inline; filename="Received text messages {}.csv"'.format(
format_date_numeric(datetime.utcnow().isoformat())
)
}
)
def get_inbox_partials(service_id): def get_inbox_partials(service_id):
if 'inbound_sms' not in current_service['permissions']: if 'inbound_sms' not in current_service['permissions']:

View File

@@ -2,6 +2,11 @@
{% from "components/message-count-label.html" import message_count_label %} {% from "components/message-count-label.html" import message_count_label %}
<div class="ajax-block-container"> <div class="ajax-block-container">
{% if messages %}
<p class="bottom-gutter-2-3 top-gutter-2-3">
<a href="{{ url_for('.inbox_download', service_id=current_service.id) }}" download="download" class="heading-small">Download these messages</a>
</p>
{% endif %}
{% call(item, row_number) list_table( {% call(item, row_number) list_table(
messages, messages,
caption="Inbox", caption="Inbox",

View File

@@ -7,6 +7,7 @@ from flask import url_for
import pytest import pytest
from bs4 import BeautifulSoup from bs4 import BeautifulSoup
from freezegun import freeze_time from freezegun import freeze_time
from datetime import datetime
from app.main.views.dashboard import ( from app.main.views.dashboard import (
get_dashboard_totals, get_dashboard_totals,
@@ -161,6 +162,10 @@ def test_inbox_showing_inbound_messages(
assert normalize_spaces(page.select('.table-show-more-link')) == ( assert normalize_spaces(page.select('.table-show-more-link')) == (
'8 messages from 5 users' '8 messages from 5 users'
) )
assert page.select_one('a[download]')['href'] == url_for(
'main.inbox_download',
service_id=SERVICE_ONE_ID,
)
def test_empty_inbox( def test_empty_inbox(
@@ -184,6 +189,7 @@ def test_empty_inbox(
assert normalize_spaces(page.select('tbody tr')) == ( assert normalize_spaces(page.select('tbody tr')) == (
'When users text your services phone number (0781239871) youll see the messages here' 'When users text your services phone number (0781239871) youll see the messages here'
) )
assert not page.select('a[download]')
@pytest.mark.parametrize('endpoint', [ @pytest.mark.parametrize('endpoint', [
@@ -246,6 +252,69 @@ def test_view_inbox_updates(
mock_get_partials.assert_called_once_with(SERVICE_ONE_ID) mock_get_partials.assert_called_once_with(SERVICE_ONE_ID)
@freeze_time("2016-07-01 13:00")
def test_download_inbox(
logged_in_client,
mock_get_inbound_sms,
):
response = logged_in_client.get(
url_for('main.inbox_download', service_id=SERVICE_ONE_ID)
)
assert response.status_code == 200
assert response.headers['Content-Type'] == (
'text/csv; '
'charset=utf-8'
)
assert response.headers['Content-Disposition'] == (
'inline; '
'filename="Received text messages 2016-07-01.csv"'
)
assert response.get_data(as_text=True) == (
'Phone number,Message,Received\r\n'
'07900900000,message-1,2016-07-01 13:00\r\n'
'07900900000,message-2,2016-07-01 12:59\r\n'
'07900900000,message-3,2016-07-01 12:59\r\n'
'07900900002,message-4,2016-07-01 10:59\r\n'
'07900900004,message-5,2016-07-01 08:59\r\n'
'07900900006,message-6,2016-07-01 06:59\r\n'
'07900900008,message-7,2016-07-01 04:59\r\n'
'07900900008,message-8,2016-07-01 04:59\r\n'
)
@freeze_time("2016-07-01 13:00")
@pytest.mark.parametrize('message_content, expected_cell', [
('=2+5', '2+5'),
('==2+5', '2+5'),
('-2+5', '2+5'),
('+2+5', '2+5'),
('@2+5', '2+5'),
('looks safe,=2+5', '"looks safe,=2+5"'),
])
def test_download_inbox_strips_formulae(
mocker,
logged_in_client,
fake_uuid,
message_content,
expected_cell,
):
mocker.patch(
'app.service_api_client.get_inbound_sms',
return_value=[{
'user_number': 'elevenchars',
'notify_number': 'foo',
'content': message_content,
'created_at': datetime.utcnow().isoformat(),
'id': fake_uuid,
}],
)
response = logged_in_client.get(
url_for('main.inbox_download', service_id=SERVICE_ONE_ID)
)
assert expected_cell in response.get_data(as_text=True).split('\r\n')[1]
def test_should_show_recent_templates_on_dashboard( def test_should_show_recent_templates_on_dashboard(
logged_in_client, logged_in_client,
mocker, mocker,