diff --git a/app/controllers/admin/forms_controller.rb b/app/controllers/admin/forms_controller.rb index e82cff51a..15f716482 100644 --- a/app/controllers/admin/forms_controller.rb +++ b/app/controllers/admin/forms_controller.rb @@ -474,6 +474,7 @@ def form_params :department, :bureau, :load_css, + :verify_csrf, :ui_truncate_text_responses, :question_text_01, :question_text_02, diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d91c8054f..d92807c7f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -254,6 +254,12 @@ def paginate(scope, default_per_page = 20) }, collection] end + # customized response for `#verify_authenticity_token` + def handle_unverified_request + render json: { messages: { submission: ["invalid CSRF authenticity token"] } }, status: :unprocessable_entity + end + + private # For Devise diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index bc7290278..dcbef51b7 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true class SubmissionsController < ApplicationController - protect_from_forgery only: [] before_action :set_form, only: %i[new create] + append_before_action :verify_authenticity_token, if: :form_requires_verification layout 'public', only: :new @@ -141,4 +141,8 @@ def submission_params permitted_fields << %i[language location_code referer page fba_directive] params.require(:submission).permit(permitted_fields) end + + def form_requires_verification + @form.verify_csrf? + end end diff --git a/app/views/admin/forms/delivery.html.erb b/app/views/admin/forms/delivery.html.erb index 1af8dd80a..c9dc0dbba 100644 --- a/app/views/admin/forms/delivery.html.erb +++ b/app/views/admin/forms/delivery.html.erb @@ -90,6 +90,14 @@
<%= render 'components/whitelist_options', f: f %> +
+
+
+ <%= f.label :verify_csrf, class: "usa-label" %> + <%= f.check_box :verify_csrf, class: "usa-checkbox" %> +
+
+

<%= f.submit (@form.persisted? ? "Update Form" : "Create Form"), class: "usa-button" %>

diff --git a/app/views/admin/forms/example.html.erb b/app/views/admin/forms/example.html.erb index e0ac85bb5..62349411f 100644 --- a/app/views/admin/forms/example.html.erb +++ b/app/views/admin/forms/example.html.erb @@ -77,6 +77,6 @@ <% end %> - + diff --git a/app/views/admin/omb_cx_reporting_collections/_form.html.erb b/app/views/admin/omb_cx_reporting_collections/_form.html.erb index 7b62bbcd7..84aa13872 100644 --- a/app/views/admin/omb_cx_reporting_collections/_form.html.erb +++ b/app/views/admin/omb_cx_reporting_collections/_form.html.erb @@ -1,4 +1,4 @@ -<%= form_with(model: omb_cx_reporting_collection, url: (omb_cx_reporting_collection.persisted? ? admin_omb_cx_reporting_collection_path(omb_cx_reporting_collection) : admin_omb_cx_reporting_collections_path), data: { turbo: false }) do |form| %> +<%= form_with(model: omb_cx_reporting_collection, url: (omb_cx_reporting_collection.persisted? ? admin_omb_cx_reporting_collection_path(omb_cx_reporting_collection) : admin_omb_cx_reporting_collections_path), local: true, data: { turbo: false }) do |form| %> <% if omb_cx_reporting_collection.errors.any? %>

<%= pluralize(omb_cx_reporting_collection.errors.count, "error") %> prohibited this omb_cx_reporting_collection from being saved:

diff --git a/app/views/components/forms/_custom.html.erb b/app/views/components/forms/_custom.html.erb index 684e427e2..0da75ce14 100644 --- a/app/views/components/forms/_custom.html.erb +++ b/app/views/components/forms/_custom.html.erb @@ -2,15 +2,16 @@
+ <%= hidden_field_tag :authenticity_token, form_authenticity_token if form.verify_csrf? -%> <%= hidden_field_tag :fba_location_code, params[:location_code] %> <% form.form_sections.each_with_index do |section, index| %>
- <% if (section != form.form_sections.first) || (section != form.form_sections.last) %> - <% if section != form.form_sections.first %> + <%- if (section != form.form_sections.first) || (section != form.form_sections.last) %> + <%- if section != form.form_sections.first %>
<%= link_to (t 'form.back'), "#previous-page", class: "usa-button previous-section", 'data-form-section-target' => "" %>
@@ -18,10 +19,10 @@ <% end %> <% end %> - <% if form.form_sections.size > 1 %> + <%- if form.form_sections.size > 1 %>

- <% if section.title.present? -%> + <%- if section.title.present? -%> <%= section.title %> - <% end %> @@ -40,9 +41,9 @@ end %> - <% if question.question_type != "hidden_field" %> + <%- if question.question_type != "hidden_field" %>
- <% if question.question_type == "text_field" %> + <%- if question.question_type == "text_field" %> <%= render 'components/forms/question_types/text_field', form: form, question: question, question_number: multi_section_question_number %> <% elsif question.question_type == "text_email_field" %> <%= render 'components/forms/question_types/text_email_field', form: form, question: question, question_number: multi_section_question_number %> @@ -78,9 +79,9 @@ <% end %>
- <% if (section != form.form_sections.first) || (section != form.form_sections.last) %> + <%- if (section != form.form_sections.first) || (section != form.form_sections.last) %>
- <% if (section == form.form_sections.first) && form.early_submission? %> + <%- if (section == form.form_sections.first) && form.early_submission? %>

<%= t 'form.answer_more_questions' %> @@ -95,7 +96,7 @@ <% end %>

<% end %> - <% if section == form.form_sections.last && !form.suppress_submit_button %> + <%- if section == form.form_sections.last && !form.suppress_submit_button %>

diff --git a/app/views/components/widget/_fba.js.erb b/app/views/components/widget/_fba.js.erb index db115919e..029e3db80 100644 --- a/app/views/components/widget/_fba.js.erb +++ b/app/views/components/widget/_fba.js.erb @@ -3,7 +3,7 @@ function FBAform(d, N) { return { - <% if form.load_css %> + <%- if form.load_css %> css: '<%= escape_javascript(render partial: "components/widget/widget", formats: :css, locals: { form: form }) %>', <% end %> formId: "<%= form.short_uuid %>", @@ -29,12 +29,12 @@ function FBAform(d, N) { init: function(options) { this.javscriptIsEnabled(); this.options = options; - <% if form.load_css %> + <%- if form.load_css %> this.loadCss(); <% end %> this.loadHtml(); - <% unless (local_assigns.has_key?(:suppress_ui) && suppress_ui) -%> - <% if form.delivery_method == "modal" -%> + <%- unless (local_assigns.has_key?(:suppress_ui) && suppress_ui) -%> + <%- if form.delivery_method == "modal" -%> this.loadButton(); <% end %> <% end %> @@ -56,7 +56,7 @@ function FBAform(d, N) { self.handleClick(event); }); }, - <% if form.load_css %> + <%- if form.load_css %> loadCss: function() { d.head.innerHTML += ''; @@ -65,15 +65,15 @@ function FBAform(d, N) { loadHtml: function() { <%# inject the form interface for inline with no wrapper %> - <% if form.delivery_method == "inline" && form.suppress_submit_button %> - <% if form.element_selector? %> + <%- if form.delivery_method == "inline" && form.suppress_submit_button %> + <%- if form.element_selector? %> if(d.getElementById('<%= form.element_selector %>') != null) { d.getElementById('<%= form.element_selector %>').innerHTML = "<%= escape_javascript render(partial: 'components/widget/no_modal', locals: { form: form }) %>"; } <% end %> <%# inject the form interface for inline with the modal wrapper %> <% elsif form.delivery_method == "inline" %> - <% if form.element_selector? %> + <%- if form.element_selector? %> if(d.getElementById('<%= form.element_selector %>') != null) { d.getElementById('<%= form.element_selector %>').innerHTML = "<%= escape_javascript render(partial: 'components/widget/modal', locals: { form: form }) %>"; } @@ -81,7 +81,7 @@ function FBAform(d, N) { <% end %> <%# inject the form interface for modal and custom-button-modal %> - <% if form.delivery_method == "modal" || form.delivery_method == "custom-button-modal" %> + <%- if form.delivery_method == "modal" || form.delivery_method == "custom-button-modal" %> this.dialogEl = document.createElement('div'); this.dialogEl.setAttribute("hidden", true); this.dialogEl.setAttribute('class', 'fba-modal'); @@ -100,8 +100,8 @@ function FBAform(d, N) { phoneElements[i].addEventListener('keyup', this.handlePhoneInput.bind(this), false); } <%# add button behaviors for custom-button-modal %> - <% if form.delivery_method == "custom-button-modal" %> - <% if form.element_selector? %> + <%- if form.delivery_method == "custom-button-modal" %> + <%- if form.element_selector? %> if(d.getElementById('<%= form.element_selector %>') != null) { d.getElementById('<%= form.element_selector %>').addEventListener('click', this.handleButtonClick.bind(this), false); } @@ -142,7 +142,7 @@ function FBAform(d, N) { alertErrorElementBody.innerHTML = ""; }, handleClick: function(e) { - <% if form.delivery_method == "modal" %> + <%- if form.delivery_method == "modal" %> if (this.dialogOpen && !e.target.closest('#fba-button') && !e.target.closest('.fba-modal-dialog')) { this.closeDialog(); } @@ -546,8 +546,13 @@ function FBAform(d, N) { xhr.open("POST", url); xhr.setRequestHeader("Content-Type", "application/json; charset=UTF-8;"); xhr.onload = callback.bind(this); + debugger xhr.send(JSON.stringify({ - "submission": params + "submission": params, + <%- if @form.verify_csrf? %> + "authenticity_token": form.querySelector("#authenticity_token") ? + form.querySelector("#authenticity_token").value : null + <% end %> })); }, pagination: function() { @@ -562,7 +567,7 @@ function FBAform(d, N) { if (!this.validateForm(currentPage)) return false; currentPage.classList.remove("visible"); currentPage.previousElementSibling.classList.add("visible"); - + // if in a modal, scroll to the top of the modal on previous button click if(document.getElementsByClassName("fba-modal")[0]) { document.getElementsByClassName("fba-modal")[0].scrollTo(0,0); diff --git a/config/environments/test.rb b/config/environments/test.rb index 74be6b76b..a30c0b96d 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -45,7 +45,7 @@ config.action_dispatch.show_exceptions = false # Disable request forgery protection in test environment. - config.action_controller.allow_forgery_protection = false + config.action_controller.allow_forgery_protection = true # Store uploaded files on the local file system in a temporary directory config.active_storage.service = :test diff --git a/db/migrate/20221213041152_add_form_csrf_flag.rb b/db/migrate/20221213041152_add_form_csrf_flag.rb new file mode 100644 index 000000000..e938eb69d --- /dev/null +++ b/db/migrate/20221213041152_add_form_csrf_flag.rb @@ -0,0 +1,7 @@ +class AddFormCsrfFlag < ActiveRecord::Migration[7.0] + def change + add_column :forms, :verify_csrf, :boolean, default: true + + Form.update_all(verify_csrf: false) + end +end diff --git a/db/schema.rb b/db/schema.rb index d2a9be98c..cec30ec34 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2022_12_07_055927) do +ActiveRecord::Schema[7.0].define(version: 2022_12_13_041152) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -235,6 +235,7 @@ t.string "notification_frequency", default: "instant" t.integer "service_id" t.integer "questions_count", default: 0 + t.boolean "verify_csrf", default: true t.index ["legacy_touchpoint_id"], name: "index_forms_on_legacy_touchpoint_id" t.index ["legacy_touchpoint_uuid"], name: "index_forms_on_legacy_touchpoint_uuid" t.index ["organization_id"], name: "index_forms_on_organization_id" diff --git a/spec/features/touchpoints_spec.rb b/spec/features/touchpoints_spec.rb index c31d601f0..1c51d0098 100644 --- a/spec/features/touchpoints_spec.rb +++ b/spec/features/touchpoints_spec.rb @@ -41,6 +41,54 @@ end end + context 'form.verify_csrf=true, but with invalid authenticity_token' do + before do + expect(form.verify_csrf).to be true + visit touchpoint_path(form) + page.execute_script("document.getElementById('authenticity_token').value = 'an invalid csrf token'") + click_button 'Submit' + end + + it 'fails the submission' do + expect(page).to have_content('Error') + expect(page).to have_content('submission invalid CSRF authenticity token') + expect(page.current_path).to eq("/touchpoints/#{form.short_uuid}/submit") # stays on same page + end + end + + context 'form.verify_csrf=true, but without authenticity_token' do + before do + expect(form.verify_csrf).to be true + visit touchpoint_path(form) + page.execute_script("document.getElementById('authenticity_token').remove()") + expect(page).to_not have_css("#authenticity_token", visible: false) + click_button 'Submit' + end + + it 'fails the submission' do + expect(page).to have_content('Error') + expect(page).to have_content('submission invalid CSRF authenticity token') + expect(page.current_path).to eq("/touchpoints/#{form.short_uuid}/submit") # stays on same page + end + end + + context 'form.verify_csrf=false' do + before do + form.update!(verify_csrf: false) + expect(form.verify_csrf).to be false + + visit touchpoint_path(form) + expect(page).to_not have_css("#authenticity_token", visible: false) + click_button 'Submit' + end + + it 'successful submission' do + expect(page).to have_content('Success') + expect(page).to have_content('Thank you. Your feedback has been received.') + expect(page.current_path).to eq("/touchpoints/#{form.short_uuid}/submit") # stays on same page + end + end + context 'custom success text' do before do form.update(success_text: "Much success. \n With a second line.")