From 16fbfe49a39a14eb30a43bc9395379cd68c38f73 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 22 Feb 2016 21:37:32 +0000
Subject: [PATCH] Fix missing CSRF token on send SMS page

As part of https://github.com/alphagov/notifications-admin/pull/187 the file
upload pattern was changed to auto-submit once a file had been picked. The
form that was submitted was, however, missing a CSRF token, as well as a submit
button for non-JS users.

This commit makes the file upload pattern self-contained, so that it will always
include a form with a CSRF token in a hidden input and a submit button, which is
then hidden when Javascript loads.
---
 app/assets/javascripts/fileUpload.js          | 12 ++++--------
 .../stylesheets/components/file-upload.scss   | 10 ++++++++++
 app/templates/components/file-upload.html     |  6 ++++--
 app/templates/views/check-sms.html            |  7 +------
 app/templates/views/send-sms.html             | 19 ++++++++-----------
 tests/app/main/views/test_sms.py              |  2 +-
 6 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/app/assets/javascripts/fileUpload.js b/app/assets/javascripts/fileUpload.js
index 4df2a1702..1ccc724e2 100644
--- a/app/assets/javascripts/fileUpload.js
+++ b/app/assets/javascripts/fileUpload.js
@@ -3,20 +3,16 @@
 
   Modules.FileUpload = function() {
 
-    let $field;
+    let $form;
 
-    this.submit = function() {
-
-      $field.parents('form').trigger('submit');
-
-    };
+    this.submit = () => $form.trigger('submit');
 
     this.start = function(component) {
 
-      $field = $('.file-upload-field', component);
+      $form = $(component);
 
       // Need to put the event on the container, not the input for it to work properly
-      $(component).on('change', '.file-upload-field', this.submit);
+      $form.on('change', '.file-upload-field', this.submit);
 
     };
 
diff --git a/app/assets/stylesheets/components/file-upload.scss b/app/assets/stylesheets/components/file-upload.scss
index d674944a5..96a6b6a3e 100644
--- a/app/assets/stylesheets/components/file-upload.scss
+++ b/app/assets/stylesheets/components/file-upload.scss
@@ -3,9 +3,15 @@
   .file-upload {
 
     &-label {
+
       @include bold-19;
       display: block;
       margin: 0 0 10px 0;
+
+      .error-message {
+        padding: 0;
+      }
+
     }
 
     &-field {
@@ -33,6 +39,10 @@
       padding-left: $gutter-half;
     }
 
+    &-submit {
+      display: none;
+    }
+
   }
 
 }
diff --git a/app/templates/components/file-upload.html b/app/templates/components/file-upload.html
index 437ce699f..0d9e65fb9 100644
--- a/app/templates/components/file-upload.html
+++ b/app/templates/components/file-upload.html
@@ -1,5 +1,5 @@
 {% macro file_upload(field, button_text="Choose file") %}
-  <div class="form-group{% if field.errors %} error{% endif %}" data-module="file-upload">
+  <form method="post" enctype="multipart/form-data" class="form-group{% if field.errors %} error{% endif %}" data-module="file-upload">
     <label class="file-upload-label" for="{{ field.name }}">
       <span class="visually-hidden">{{ field.label }}</span>
       {% if hint %}
@@ -20,5 +20,7 @@
       {{ button_text }}
     </label>
     <label class="file-upload-filename" for="{{ field.name }}"></label>
-  </div>
+    <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
+    <input type="submit" class="file-upload-submit" value="Submit" />
+  </form>
 {% endmacro %}
diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html
index c41c4759f..4b1354847 100644
--- a/app/templates/views/check-sms.html
+++ b/app/templates/views/check-sms.html
@@ -45,12 +45,7 @@
       <a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
     </form>
   {% else %}
-    <form method="post" action="{{ url_for('.send_sms', service_id=service_id, template_id=template.id) }}" enctype="multipart/form-data">
-      {{file_upload(form.file, button_text='Choose a CSV file')}}
-      {{ page_footer(
-        "Upload"
-      ) }}
-    </form>
+    {{file_upload(form.file, button_text='Upload a CSV file')}}
   {% endif %}
 
   {% call(item) list_table(
diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html
index 32be81c72..7e44bad16 100644
--- a/app/templates/views/send-sms.html
+++ b/app/templates/views/send-sms.html
@@ -12,19 +12,16 @@
 
   <h1 class="heading-large">Add recipients</h1>
 
-  <form method="POST" enctype="multipart/form-data">
-
-    <div class="grid-row">
-      <div class="column-two-thirds">
-        {{ sms_message(template.formatted_as_markup) }}
-      </div>
+  <div class="grid-row">
+    <div class="column-two-thirds">
+      {{ sms_message(template.formatted_as_markup) }}
     </div>
+  </div>
 
-    <p>
-      <a href="{{ url_for('.get_example_csv', service_id=service_id, template_id=template.id) }}">Download an example CSV file</a>
-    </p>
+  <p>
+    <a href="{{ url_for('.get_example_csv', service_id=service_id, template_id=template.id) }}">Download an example CSV file</a>
+  </p>
 
-    {{file_upload(form.file, button_text='Upload a CSV file')}}
+  {{file_upload(form.file, button_text='Upload a CSV file')}}
 
-  </form>
 {% endblock %}
diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py
index 573e41d14..0c50631dc 100644
--- a/tests/app/main/views/test_sms.py
+++ b/tests/app/main/views/test_sms.py
@@ -66,7 +66,7 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(app_,
         assert 'Your CSV file contained missing or invalid data' in content
         assert '+44 123' in content
         assert '+44 456' in content
-        assert 'Choose a CSV file' in content
+        assert 'Upload a CSV file' in content
 
 
 @moto.mock_s3