Don’t populate support form with arbitrary text

I don’t think it’s a massive risk (we’re certainly mitigating against
any XSS), but having a page on a GOV.UK domain where you can prefill
text on the page from a query string probably isn’t great.

So this commit restricts prefilling the support form to a set of
named questions.

app/main/views/feedback.py

@@ -13,11 +13,26 @@ from app import (
 )
 from app.main import main
 from app.main.forms import Feedback, Problem, SupportType, Triage
+from app.utils import AgreementInfo
 
 QUESTION_TICKET_TYPE = 'ask-question-give-feedback'
 PROBLEM_TICKET_TYPE = "report-problem"
 
 
+def get_prefilled_message():
+    agreement_info = AgreementInfo.from_current_user()
+    return {
+        'agreement': (
+            agreement_info.as_request_for_agreement()
+        ),
+        'agreement-with-owner': (
+            agreement_info.as_request_for_agreement(with_owner=True)
+        ),
+    }.get(
+        request.args.get('body'), ''
+    )
+
+
 @main.route('/feedback', methods=['GET'])
 def old_feedback():
     return redirect(url_for('.support'))
@@ -135,7 +150,7 @@ def feedback(ticket_type):
         return redirect(url_for('.thanks', urgent=urgent, anonymous=anonymous))
 
     if not form.feedback.data:
-        form.feedback.data = request.args.get('body', '')
+        form.feedback.data = get_prefilled_message()
 
     return render_template(
         'views/support/{}.html'.format(ticket_type),

app/templates/views/pricing.html

@@ -125,7 +125,7 @@
     <h2 class="heading-medium" id="paying">How to pay</h2>
     <p>You can find details of how to pay for Notify in our data sharing and financial agreement.</p>
     <p>
-      <a href="{{url_for('.feedback', ticket_type='ask-question-give-feedback', body='Please send me a copy of the GOV.UK Notify data sharing and financial agreement.')}}">Contact us</a> to get a copy of the agreement
+      <a href="{{url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement')}}">Contact us</a> to get a copy of the agreement
       {% if agreement_info.agreement_signed %}
         ({{ agreement_info.owner }} has already accepted it).
       {% else %}

app/templates/views/terms-of-use.html

@@ -28,10 +28,10 @@ Terms of use
         {% if agreement_info.owner %}
           ({{ agreement_info.owner }})
           must also accept our data sharing and financial agreement.
-          <a href="{{ url_for('.feedback', ticket_type='ask-question-give-feedback', body='Please send me a copy of the GOV.UK Notify data sharing and financial agreement for {} to sign.'.format(agreement_info.owner)) }}">Contact us</a> to get a copy.
+          <a href="{{ url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement-with-owner') }}">Contact us</a> to get a copy.
         {% else %}
           must also accept our data sharing and financial agreement.
-          <a href="{{ url_for('.feedback', ticket_type='ask-question-give-feedback', body='Please send me a copy of the GOV.UK Notify data sharing and financial agreement.') }}">Contact us</a> to get a copy.
+          <a href="{{ url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement') }}">Contact us</a> to get a copy.
         {% endif %}
       </p>
     {% endif %}

app/utils.py

@@ -464,6 +464,17 @@ class AgreementInfo:
         else:
             return 'Can’t tell'
 
+    def as_request_for_agreement(self, with_owner=False):
+        if with_owner and self.owner:
+            return (
+                'Please send me a copy of the GOV.UK Notify data sharing '
+                'and financial agreement for {} to sign.'.format(self.owner)
+            )
+        return (
+            'Please send me a copy of the GOV.UK Notify data sharing '
+            'and financial agreement.'
+        )
+
     @staticmethod
     def get_matching_function(email_address_or_domain):
 

tests/app/main/views/test_feedback.py

@@ -7,6 +7,7 @@ from flask import url_for
 from freezegun import freeze_time
 from notifications_utils.clients import DeskproError
 from tests.conftest import (
+    active_user_with_permissions,
     mock_get_services,
     mock_get_services_with_no_services,
     mock_get_services_with_one_service,
@@ -85,29 +86,56 @@ def test_get_feedback_page(client, ticket_type, expected_status_code):
     assert response.status_code == expected_status_code
 
 
+@pytest.mark.parametrize('prefilled_body, expected_textarea', [
+    (
+        'agreement',
+        (
+            'Please send me a copy of the GOV.UK Notify data sharing '
+            'and financial agreement.'
+        )
+    ),
+    (
+        'agreement-with-owner',
+        (
+            'Please send me a copy of the GOV.UK Notify data sharing '
+            'and financial agreement for Marine Management '
+            'Organisation to sign.'
+        )
+    ),
+    (
+        'foo',
+        ''
+    ),
+])
 @freeze_time('2016-12-12 12:00:00.000000')
 def test_get_feedback_page_with_prefilled_body(
     client_request,
     mocker,
+    fake_uuid,
+    prefilled_body,
+    expected_textarea,
 ):
+    user = active_user_with_permissions(fake_uuid)
+    user.email_address = 'test@marinemanagement.org.uk'
+    mocker.patch('app.user_api_client.get_user', return_value=user)
     mock_post = mocker.patch('app.main.views.feedback.deskpro_client.create_ticket')
     page = client_request.get(
         'main.feedback',
         ticket_type=QUESTION_TICKET_TYPE,
-        body='Please send cat pictures <script>alert("foo");</script>',
+        body=prefilled_body,
     )
     assert page.select_one('textarea').text == (
-        'Please send cat pictures <script>alert("foo");</script>'
+        expected_textarea
     )
     client_request.post(
         'main.feedback',
         ticket_type=QUESTION_TICKET_TYPE,
-        body='Please send cat pictures <script>alert("foo");</script>',
+        body='agreement',
         _data={'feedback': 'blah', 'name': 'Example', 'email_address': 'test@example.com'}
     )
     message = mock_post.call_args[1]['message']
     assert message.endswith('blah')
-    assert 'cat pictures' not in message
+    assert 'Please send' not in message
 
 
 @freeze_time('2016-12-12 12:00:00.000000')