From 7c319aaf555a0db762129ff641f09250ae2829c5 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 23 Dec 2024 12:04:28 -0500
Subject: [PATCH 1/4] Authentication header xsd validations

---
 app/forms/state_file/state_id_concern.rb            | 3 +--
 app/lib/submission_builder/authentication_header.rb | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/app/forms/state_file/state_id_concern.rb b/app/forms/state_file/state_id_concern.rb
index 4cd63724c0..98c757593b 100644
--- a/app/forms/state_file/state_id_concern.rb
+++ b/app/forms/state_file/state_id_concern.rb
@@ -28,8 +28,7 @@ module StateIdConcern
       validates :id_type, presence: true
       validates :id_number,
                 presence: true,
-                # Seems to just be for New York
-                # alphanumeric: true, length: {is: 9},
+                length: { in: 1..40 },
                 if: -> { id_type != "no_id" }
     end
   end
diff --git a/app/lib/submission_builder/authentication_header.rb b/app/lib/submission_builder/authentication_header.rb
index 301dcec31e..636c90224a 100644
--- a/app/lib/submission_builder/authentication_header.rb
+++ b/app/lib/submission_builder/authentication_header.rb
@@ -42,7 +42,7 @@ def document
               xml.IPv6AddressTxt device_info&.ip_address if device_info&.ip_address&.ipv6?
             end
             xml.IPTs datetime_type(device_info&.updated_at)
-            xml.DeviceId device_info&.device_id || 'AB' * 20
+            xml.DeviceId /^[A-F0-9]{40}$/.match?(device_info&.device_id) ? device_info&.device_id : 'AB' * 20
             xml.DeviceTypeCd 'Browser-based'
           end
           xml.TotActiveTimePrepSubmissionTs state_file_total_preparation_submission_minutes

From a9c96b552d384df863b5a38f8e22a6fda65f2f64 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 23 Dec 2024 18:40:59 -0500
Subject: [PATCH 2/4] Fix issue where wasn't recognizing more than one question
 with followup option when page loads

---
 app/javascript/lib/honeycrisp.js              | 16 +++++----
 .../primary_state_id/_state_id.html.erb       | 36 +++++++++----------
 2 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/app/javascript/lib/honeycrisp.js b/app/javascript/lib/honeycrisp.js
index 67e9db227e..e6defc98fb 100644
--- a/app/javascript/lib/honeycrisp.js
+++ b/app/javascript/lib/honeycrisp.js
@@ -103,13 +103,17 @@ var followUpQuestion = (function () {
 
                 // set initial state of follow-ups based on the page
                 $(this).find('input').each(function (index, input) {
-                    if ($(this).attr('data-follow-up') != null) {
-                        if ($(this).is(':checked')) {
-                            $($(this).attr('data-follow-up')).find('input, select').attr('disabled', false);
-                            $($(this).attr('data-follow-up')).show();
+                    var $input = $(this);
+                    var followUpSelector = $input.attr('data-follow-up');
+                    if (followUpSelector) {
+                        if ($input.is(':checked')) {
+                            $(followUpSelector).find('input, select').attr('disabled', false);
+                            $(followUpSelector).show();
                         } else {
-                            $($(this).attr('data-follow-up')).find('input, select').attr('disabled', true);
-                            $($(this).attr('data-follow-up')).hide();
+                            if ($('[data-follow-up="' + followUpSelector + '"]:checked').length === 0) {
+                                $(followUpSelector).find('input, select').attr('disabled', true);
+                                $(followUpSelector).hide();
+                            }
                         }
                     }
                 });
diff --git a/app/views/state_file/questions/primary_state_id/_state_id.html.erb b/app/views/state_file/questions/primary_state_id/_state_id.html.erb
index aa065bb1ca..853f6f88a9 100644
--- a/app/views/state_file/questions/primary_state_id/_state_id.html.erb
+++ b/app/views/state_file/questions/primary_state_id/_state_id.html.erb
@@ -3,22 +3,20 @@
     <%= hidden_field_tag "return_to_review", params[:return_to_review] %>
   <% end %>
   <div class="question-with-follow-up spacing-below-25">
-    <div class="question-with-follow-up__question spacing-below-15">
-      <div class="white-group">
-        <div class="form-question spacing-below-15">
-          <strong><%= t("state_file.questions.primary_state_id.state_id.id_type_question.label") %></strong>
-        </div>
-        <%=
-          f.cfa_radio_set(
-            :id_type,
-            collection: [
-              { value: :driver_license, label: t("state_file.questions.primary_state_id.state_id.id_type_question.drivers_license"), input_html: { "data-follow-up": "#id-details-fields" } },
-              { value: :dmv_bmv, label: (options[:dmv_bmv_label] || t("state_file.questions.primary_state_id.state_id.id_type_question.dmv")), input_html: { "data-follow-up": "#id-details-fields" } },
-              { value: :no_id, label: (options[:no_id_label] || t("state_file.questions.primary_state_id.state_id.id_type_question.no_id")) },
-            ]
-          )
-        %>
+    <div class="question-with-follow-up__question spacing-below-15 white-group">
+      <div class="form-question spacing-below-15">
+        <strong><%= t("state_file.questions.primary_state_id.state_id.id_type_question.label") %></strong>
       </div>
+      <%=
+        f.cfa_radio_set(
+          :id_type,
+          collection: [
+            { value: :driver_license, label: t("state_file.questions.primary_state_id.state_id.id_type_question.drivers_license"), input_html: { "data-follow-up": "#id-details-fields" } },
+            { value: :dmv_bmv, label: (options[:dmv_bmv_label] || t("state_file.questions.primary_state_id.state_id.id_type_question.dmv")), input_html: { "data-follow-up": "#id-details-fields" } },
+            { value: :no_id, label: (options[:no_id_label] || t("state_file.questions.primary_state_id.state_id.id_type_question.no_id")) },
+          ]
+        )
+      %>
     </div>
 
     <div class="question-with-follow-up__follow-up" id="id-details-fields">
@@ -45,7 +43,7 @@
         <div onchange="$('#first_3_doc_num_container')[event.target.value == 'NY' ? 'show' : 'hide']()">
           <%= f.cfa_select(:state, t("state_file.questions.primary_state_id.state_id.id_details.issue_state"), States.name_value_pairs, include_blank: true) %>
         </div>
-        <div id="first_3_doc_num_container"  style="display: <%=options[:include_first_three_doc_num] ? 'block' : 'none'%>">
+        <div id="first_3_doc_num_container" style="display: <%= options[:include_first_three_doc_num] ? 'block' : 'none' %>">
           <%= f.cfa_input_field(:first_three_doc_num, t("state_file.questions.primary_state_id.state_id.id_details.first_three_doc_num_html"), classes: ["form-width--short"]) %>
         </div>
         <% if options[:info_link] %>
@@ -65,7 +63,7 @@
       document.addEventListener("DOMContentLoaded", function () {
           // Function to clear selections in fields_to_clear
           function clearFieldsToClear() {
-              $('#fields_to_clear select').each(function() {
+              $('#fields_to_clear select').each(function () {
                   $(this).val('');
               });
           }
@@ -76,14 +74,14 @@
           }
 
           // Attach a change event listener to the selects within fields_to_clear
-          $('#fields_to_clear select').change(function() {
+          $('#fields_to_clear select').change(function () {
               if ($('#checkbox input').is(':checked')) {
                   clearFieldsToClear();
               }
           });
 
           // Attach a change event listener to the checkbox as well
-          $('#checkbox input').change(function() {
+          $('#checkbox input').change(function () {
               if (this.checked) {
                   clearFieldsToClear();
               }

From 06690c38e41bd6f4fa1752c33d51491ca79bbaa6 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 23 Dec 2024 19:00:13 -0500
Subject: [PATCH 3/4] protext against XSS

---
 app/javascript/lib/honeycrisp.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/javascript/lib/honeycrisp.js b/app/javascript/lib/honeycrisp.js
index e6defc98fb..9e7faad4f1 100644
--- a/app/javascript/lib/honeycrisp.js
+++ b/app/javascript/lib/honeycrisp.js
@@ -105,7 +105,7 @@ var followUpQuestion = (function () {
                 $(this).find('input').each(function (index, input) {
                     var $input = $(this);
                     var followUpSelector = $input.attr('data-follow-up');
-                    if (followUpSelector) {
+                    if (followUpSelector && /^[a-zA-Z0-9_\-#\.]+$/.test(followUpSelector)) {
                         if ($input.is(':checked')) {
                             $(followUpSelector).find('input, select').attr('disabled', false);
                             $(followUpSelector).show();

From d04d5a2d7f2569552c207d50faf338a5505762d4 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 24 Dec 2024 16:51:32 -0500
Subject: [PATCH 4/4] Add test for device id and add comment for follow-up js
 code

---
 app/javascript/lib/honeycrisp.js              |  3 +-
 .../authentication_header.rb                  |  2 +-
 .../states/authentication_header_spec.rb      | 49 +++++++++++++++++--
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/app/javascript/lib/honeycrisp.js b/app/javascript/lib/honeycrisp.js
index 9e7faad4f1..2a1d3c6017 100644
--- a/app/javascript/lib/honeycrisp.js
+++ b/app/javascript/lib/honeycrisp.js
@@ -105,13 +105,14 @@ var followUpQuestion = (function () {
                 $(this).find('input').each(function (index, input) {
                     var $input = $(this);
                     var followUpSelector = $input.attr('data-follow-up');
-                    if (followUpSelector && /^[a-zA-Z0-9_\-#\.]+$/.test(followUpSelector)) {
+                    if (followUpSelector && /^[a-zA-Z0-9_\-#\.]+$/.test(followUpSelector)) { // protects against XSS through DOM
                         if ($input.is(':checked')) {
                             $(followUpSelector).find('input, select').attr('disabled', false);
                             $(followUpSelector).show();
                         } else {
                             if ($('[data-follow-up="' + followUpSelector + '"]:checked').length === 0) {
                                 $(followUpSelector).find('input, select').attr('disabled', true);
+                                // two question inputs can refer to the same followup
                                 $(followUpSelector).hide();
                             }
                         }
diff --git a/app/lib/submission_builder/authentication_header.rb b/app/lib/submission_builder/authentication_header.rb
index 636c90224a..58b6137245 100644
--- a/app/lib/submission_builder/authentication_header.rb
+++ b/app/lib/submission_builder/authentication_header.rb
@@ -30,7 +30,7 @@ def document
               xml.IPv6AddressTxt device_info&.ip_address if device_info&.ip_address&.ipv6?
             end
             xml.IPTs datetime_type(device_info&.updated_at)
-            xml.DeviceId device_info&.device_id || 'AB' * 20
+            xml.DeviceId /^[A-F0-9]{40}$/.match?(device_info&.device_id) ? device_info&.device_id : 'AB' * 20
             xml.DeviceTypeCd 'Browser-based'
             xml.EmailAddressTxt email_from_intake_or_df
             xml.USCellPhoneNum phone_number if phone_number.present?
diff --git a/spec/lib/submission_builder/ty2022/states/authentication_header_spec.rb b/spec/lib/submission_builder/ty2022/states/authentication_header_spec.rb
index cd2cd879fb..ac98d3b4e1 100644
--- a/spec/lib/submission_builder/ty2022/states/authentication_header_spec.rb
+++ b/spec/lib/submission_builder/ty2022/states/authentication_header_spec.rb
@@ -22,7 +22,7 @@
     end
 
     context "when a New York drivers license is defined" do
-      let(:state_id) { create(:state_id)}
+      let(:state_id) { create(:state_id) }
       let(:intake) { create(:state_file_ny_intake, primary_state_id: state_id) }
       let(:submission) { create(:efile_submission, data_source: intake) }
 
@@ -43,7 +43,7 @@
     end
 
     context "when a drivers license which does not expire is defined" do
-      let(:state_id) { create(:state_id, :non_expiring)}
+      let(:state_id) { create(:state_id, :non_expiring) }
       let(:intake) { create(:state_file_ny_intake, primary_state_id: state_id) }
       let(:submission) { create(:efile_submission, data_source: intake) }
 
@@ -55,7 +55,7 @@
     end
 
     context "when a non New York drivers license is defined" do
-      let(:state_id) { create(:state_id, :non_ny)}
+      let(:state_id) { create(:state_id, :non_ny) }
       let(:intake) { create(:state_file_ny_intake, primary_state_id: state_id) }
       let(:submission) { create(:efile_submission, data_source: intake) }
 
@@ -67,7 +67,7 @@
     end
 
     context "when a New York state issued id is defined" do
-      let(:state_id) { create(:state_id, :state_issued_id)}
+      let(:state_id) { create(:state_id, :state_issued_id) }
       let(:intake) { create(:state_file_ny_intake, primary_state_id: state_id) }
       let(:submission) { create(:efile_submission, data_source: intake) }
 
@@ -90,7 +90,7 @@
   end
 
   describe '#refund_disbursement' do
-    let(:state_id) { create(:state_id, :state_issued_id)}
+    let(:state_id) { create(:state_id, :state_issued_id) }
     let(:intake) { create(:state_file_ny_intake, primary_state_id: state_id) }
     let(:submission) { create(:efile_submission, data_source: intake) }
 
@@ -197,4 +197,43 @@
     end
   end
 
+  describe "device id" do
+    let(:intake) { create(:state_file_az_intake) }
+    let(:submission) { create(:efile_submission, data_source: intake.reload) }
+    let!(:initial_efile_device_info) { create :state_file_efile_device_info, :initial_creation, :filled, intake: intake, device_id: device_id }
+    let!(:submission_efile_device_info) { create :state_file_efile_device_info, :submission, :filled, intake: intake, device_id: device_id }
+    let(:device_id) { "AA" * 20 }
+
+    context "when the device id is alphanumeric 40 character count" do
+      let(:device_id) { "7BA1E530D6503F380F1496A47BEB6F33E40403D1" }
+      it "sends submitted device id" do
+        doc = SubmissionBuilder::AuthenticationHeader.new(submission).document
+        expect(doc.at("DeviceId").text).to eq "7BA1E530D6503F380F1496A47BEB6F33E40403D1"
+      end
+    end
+
+    context "when the device id doesn't have a character count of 40" do
+      let(:device_id) { "abba" }
+      it "sends the default device id" do
+        doc = SubmissionBuilder::AuthenticationHeader.new(submission).document
+        expect(doc.at("DeviceId").text).to eq 'AB' * 20
+      end
+    end
+
+    context "when the device id is not capitalized alphanumeric characters" do
+      let(:device_id) { "aa" * 20 }
+      it "sends the default device id" do
+        doc = SubmissionBuilder::AuthenticationHeader.new(submission).document
+        expect(doc.at("DeviceId").text).to eq 'AB' * 20
+      end
+    end
+
+    context "when the device id is not alphanumeric characters" do
+      let(:device_id) { "A*" * 20 }
+      it "sends the default device id" do
+        doc = SubmissionBuilder::AuthenticationHeader.new(submission).document
+        expect(doc.at("DeviceId").text).to eq 'AB' * 20
+      end
+    end
+  end
 end
\ No newline at end of file