From 58d8519bb58ba0943f367521b715505a7abf575d Mon Sep 17 00:00:00 2001 From: Jacob Penner <161746194+pennja@users.noreply.github.com> Date: Tue, 26 Mar 2024 08:58:25 -0400 Subject: [PATCH] [1115] poc: add logic to verify a PDF was properly stamped (#16051) * add logic to verify a PDF was properly stamped * update existing pdf_stamper spec to allow additional contextualized tests * add back original Gemfile.lock * further simplification of spec logic * wrap stamp calls in verified_stamp * add test coverage for new logic * rename verify_stamp method to be shorter and generalized * add nil check for multistamp signature_text * add intermediary verification step to stamping * update test coverage for new and existing logic * remove intermediary stamp verification * update method signature and error to include more generic messaging * fix broken tests * resolve merge * fix merge issue --- .../services/simple_forms_api/pdf_stamper.rb | 51 +++--- ...rm_with_dangerous_characters_21P_0847.json | 3 +- ...orm_with_dangerous_characters_21_0845.json | 5 +- ...rm_with_dangerous_characters_21_10210.json | 5 +- ...orm_with_dangerous_characters_21_4142.json | 3 +- ...m_with_dangerous_characters_unhandled.json | 3 +- .../spec/fixtures/form_json/vba_21_0845.json | 3 +- .../fixtures/form_json/vba_21_10210-min.json | 5 +- .../spec/fixtures/form_json/vba_21_10210.json | 5 +- .../fixtures/form_json/vba_21p_0847-min.json | 3 +- .../spec/fixtures/form_json/vba_21p_0847.json | 3 +- .../spec/services/pdf_filler_spec.rb | 23 +-- .../spec/services/pdf_stamper_spec.rb | 151 ++++++++++++++++-- 13 files changed, 208 insertions(+), 55 deletions(-) diff --git a/modules/simple_forms_api/app/services/simple_forms_api/pdf_stamper.rb b/modules/simple_forms_api/app/services/simple_forms_api/pdf_stamper.rb index 26999cb00c3..a0f8d61bcb7 100644 --- a/modules/simple_forms_api/app/services/simple_forms_api/pdf_stamper.rb +++ b/modules/simple_forms_api/app/services/simple_forms_api/pdf_stamper.rb @@ -26,7 +26,7 @@ def self.stamp_pdf(stamped_template_path, form, current_loa) end stamp_text = SUBMISSION_TEXT + current_time desired_stamps = [[10, 10, stamp_text]] - stamp(desired_stamps, stamped_template_path, auth_text, text_only: false) + verify(stamped_template_path) { stamp(desired_stamps, stamped_template_path, auth_text, text_only: false) } stamp_submission_date(stamped_template_path, form.submission_date_config) end @@ -34,7 +34,7 @@ def self.stamp_pdf(stamped_template_path, form, current_loa) def self.stamp107959f1(stamped_template_path, form) desired_stamps = [[26, 82.5, form.data['statement_of_truth_signature']]] append_to_stamp = false - stamp(desired_stamps, stamped_template_path, append_to_stamp) + verify(stamped_template_path) { stamp(desired_stamps, stamped_template_path, append_to_stamp) } end def self.stamp264555(stamped_template_path, form) @@ -55,7 +55,7 @@ def self.stamp214142(stamped_template_path, form) { type: :new_page } ] - multistamp(stamped_template_path, signature_text, page_configuration) + verified_multistamp(stamped_template_path, signature_text, page_configuration) # This is a one-off case where we need to stamp a date on the first page of 21-4142 when resubmitting if form.data['in_progress_form_created_at'] @@ -74,7 +74,7 @@ def self.stamp214142_date_stamp_for_resubmission(stamped_template_path, date_tit { type: :new_page } ] - multistamp(stamped_template_path, date_title, page_configuration, 12) + verified_multistamp(stamped_template_path, date_title, page_configuration, 12) page_configuration = [ { type: :text, position: date_text_stamp_position }, @@ -82,7 +82,7 @@ def self.stamp214142_date_stamp_for_resubmission(stamped_template_path, date_tit { type: :new_page } ] - multistamp(stamped_template_path, date_text, page_configuration, 12) + verified_multistamp(stamped_template_path, date_text, page_configuration, 12) end def self.stamp2110210(stamped_template_path, form) @@ -94,7 +94,7 @@ def self.stamp2110210(stamped_template_path, form) { type: :text, position: desired_stamps[0] } ] - multistamp(stamped_template_path, signature_text, page_configuration) + verified_multistamp(stamped_template_path, signature_text, page_configuration) end def self.stamp210845(stamped_template_path, form) @@ -106,7 +106,7 @@ def self.stamp210845(stamped_template_path, form) { type: :text, position: desired_stamps[0] } ] - multistamp(stamped_template_path, signature_text, page_configuration) + verified_multistamp(stamped_template_path, signature_text, page_configuration) end def self.stamp21p0847(stamped_template_path, form) @@ -117,7 +117,7 @@ def self.stamp21p0847(stamped_template_path, form) { type: :text, position: desired_stamps[0] } ] - multistamp(stamped_template_path, signature_text, page_configuration) + verified_multistamp(stamped_template_path, signature_text, page_configuration) end def self.stamp210972(stamped_template_path, form) @@ -129,7 +129,7 @@ def self.stamp210972(stamped_template_path, form) { type: :text, position: desired_stamps[0] } ] - multistamp(stamped_template_path, signature_text, page_configuration) + verified_multistamp(stamped_template_path, signature_text, page_configuration) end def self.stamp210966(stamped_template_path, form) @@ -140,7 +140,7 @@ def self.stamp210966(stamped_template_path, form) { type: :text, position: desired_stamps[0] } ] - multistamp(stamped_template_path, signature_text, page_configuration) + verified_multistamp(stamped_template_path, signature_text, page_configuration) end def self.stamp2010207(stamped_template_path, form) @@ -162,7 +162,7 @@ def self.stamp2010207(stamped_template_path, form) { type: :text, position: desired_stamps[0] } ] - multistamp(stamped_template_path, signature_text, page_configuration) + verified_multistamp(stamped_template_path, signature_text, page_configuration) end def self.stamp4010007_uuid(uuid) @@ -173,7 +173,7 @@ def self.stamp4010007_uuid(uuid) { type: :text, position: desired_stamps[0] } ] - multistamp(stamped_template_path, uuid, page_configuration, 7) + verified_multistamp(stamped_template_path, uuid, page_configuration, 7) end def self.multistamp(stamped_template_path, signature_text, page_configuration, font_size = 16) @@ -200,9 +200,8 @@ def self.multistamp(stamped_template_path, signature_text, page_configuration, f def self.stamp(desired_stamps, stamped_template_path, append_to_stamp, text_only: true) current_file_path = stamped_template_path desired_stamps.each do |x, y, text| - out_path = CentralMail::DatestampPdf.new(current_file_path, append_to_stamp:).run(text:, x:, y:, text_only:, - size: 9) - current_file_path = out_path + datestamp_instance = CentralMail::DatestampPdf.new(current_file_path, append_to_stamp:) + current_file_path = datestamp_instance.run(text:, x:, y:, text_only:, size: 9) end File.rename(current_file_path, stamped_template_path) end @@ -225,16 +224,32 @@ def self.stamp_submission_date(stamped_template_path, config) page_configuration = default_page_configuration page_configuration[config[:page_number]] = { type: :text, position: date_title_stamp_position } - multistamp(stamped_template_path, SUBMISSION_DATE_TITLE, page_configuration, 12) + verified_multistamp(stamped_template_path, SUBMISSION_DATE_TITLE, page_configuration, 12) page_configuration = default_page_configuration page_configuration[config[:page_number]] = { type: :text, position: date_text_stamp_position } - multistamp(stamped_template_path, Time.current.in_time_zone('UTC').strftime('%H:%M %Z %D'), page_configuration, - 12) + current_time = Time.current.in_time_zone('UTC').strftime('%H:%M %Z %D') + verified_multistamp(stamped_template_path, current_time, page_configuration, 12) end end + def self.verify(template_path) + orig_size = File.size(template_path) + yield + stamped_size = File.size(template_path) + + raise StandardError, 'The PDF remained unchanged upon stamping.' unless stamped_size > orig_size + rescue => e + raise StandardError, "An error occurred while verifying stamp: #{e}" + end + + def self.verified_multistamp(stamped_template_path, stamp_text, page_configuration, *) + raise StandardError, 'The provided stamp content was empty.' if stamp_text.blank? + + verify(stamped_template_path) { multistamp(stamped_template_path, stamp_text, page_configuration, *) } + end + def self.default_page_configuration [ { type: :new_page }, diff --git a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21P_0847.json b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21P_0847.json index d274b0ddb0f..805624bc616 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21P_0847.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21P_0847.json @@ -30,5 +30,6 @@ "relationship_to_veteran": "other", "otherRelationship_to_veteran": "friend of a friend" }, - "additional_information": "Lots of \"extra\" stuff here" + "additional_information": "Lots of \"extra\" stuff here", + "statement_of_truth_signature": "John Veteran" } diff --git a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_0845.json b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_0845.json index 1f44b104b94..2e6769e34c6 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_0845.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_0845.json @@ -44,5 +44,6 @@ "release_duration": "untilDate", "release_end_date": "2033-06-16", "security_question": "motherBirthplace", - "security_answer": "Las Vegas, NV" -} \ No newline at end of file + "security_answer": "Las Vegas, NV", + "statement_of_truth_signature": "John Veteran" +} diff --git a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_10210.json b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_10210.json index 2a60ad76f32..c8f7f3e707b 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_10210.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_10210.json @@ -53,5 +53,6 @@ }, "witness_other_relationship_to_claimant": "Other text", "claimant_type": "non-veteran", - "claim_ownership": "third-party" -} \ No newline at end of file + "claim_ownership": "third-party", + "statement_of_truth_signature": "John Veteran" +} diff --git a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_4142.json b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_4142.json index 9ecd0ea59eb..fee3abb9e15 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_4142.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_21_4142.json @@ -138,5 +138,6 @@ "preparer_organization": "Top Org", "court_appointment_info": "Representing \"court stuff\" like...Representing court stuff like...Representing court stuff like...Representing court stuff like...Representing court stuff like...Representing court stuff like...", "relationship_to_veteran": "Veteran Service Officer" - } + }, + "statement_of_truth_signature": "John Veteran" } diff --git a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_unhandled.json b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_unhandled.json index 264454c7afb..84a9daaf2a1 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_unhandled.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/form_with_dangerous_characters_unhandled.json @@ -59,5 +59,6 @@ "postal_code": "54890" } }, - "remarks": "Lengthy \"remarks\" here \nabout what is needed\tand such" + "remarks": "Lengthy \"remarks\" here \nabout what is needed\tand such", + "statement_of_truth_signature": "John Veteran" } diff --git a/modules/simple_forms_api/spec/fixtures/form_json/vba_21_0845.json b/modules/simple_forms_api/spec/fixtures/form_json/vba_21_0845.json index f70216a3970..61d5e28265a 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/vba_21_0845.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/vba_21_0845.json @@ -36,5 +36,6 @@ "release_duration": "untilDate", "release_end_date": "2033-06-16", "security_question": "motherBirthplace", - "security_answer": "Las Vegas, NV" + "security_answer": "Las Vegas, NV", + "statement_of_truth_signature": "John M Veteran" } diff --git a/modules/simple_forms_api/spec/fixtures/form_json/vba_21_10210-min.json b/modules/simple_forms_api/spec/fixtures/form_json/vba_21_10210-min.json index cac1eca116f..4ed3b48f7fa 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/vba_21_10210-min.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/vba_21_10210-min.json @@ -16,5 +16,6 @@ "state": "CA", "postal_code": "12345" }, - "veteran_phone": "555-555-5557" -} \ No newline at end of file + "veteran_phone": "555-555-5557", + "statement_of_truth_signature": "Arthur C Preparer" +} diff --git a/modules/simple_forms_api/spec/fixtures/form_json/vba_21_10210.json b/modules/simple_forms_api/spec/fixtures/form_json/vba_21_10210.json index 58faea2d302..a81b640091f 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/vba_21_10210.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/vba_21_10210.json @@ -53,5 +53,6 @@ }, "witness_other_relationship_to_claimant": "Other text", "claimant_type": "non-veteran", - "claim_ownership": "third-party" -} \ No newline at end of file + "claim_ownership": "third-party", + "statement_of_truth_signature": "Joe Center Claimant" +} diff --git a/modules/simple_forms_api/spec/fixtures/form_json/vba_21p_0847-min.json b/modules/simple_forms_api/spec/fixtures/form_json/vba_21p_0847-min.json index 83febd1e180..8f5651e8163 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/vba_21p_0847-min.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/vba_21p_0847-min.json @@ -22,5 +22,6 @@ "veteran_ssn": "999442222", "relationship_to_deceased_claimant": { "relationship_to_veteran": "spouse" - } + }, + "statement_of_truth_signature": "Arthur C Preparer" } diff --git a/modules/simple_forms_api/spec/fixtures/form_json/vba_21p_0847.json b/modules/simple_forms_api/spec/fixtures/form_json/vba_21p_0847.json index ac311237602..281ef545f63 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/vba_21p_0847.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/vba_21p_0847.json @@ -30,5 +30,6 @@ "relationship_to_veteran": "other", "otherRelationship_to_veteran": "friend of a friend" }, - "additional_information": "Lots of extra stuff here" + "additional_information": "Lots of extra stuff here", + "statement_of_truth_signature": "Arthur C Preparer" } diff --git a/modules/simple_forms_api/spec/services/pdf_filler_spec.rb b/modules/simple_forms_api/spec/services/pdf_filler_spec.rb index 9e1c40752b1..1cfb14fb5fe 100644 --- a/modules/simple_forms_api/spec/services/pdf_filler_spec.rb +++ b/modules/simple_forms_api/spec/services/pdf_filler_spec.rb @@ -5,18 +5,21 @@ describe SimpleFormsApi::PdfFiller do def self.test_pdf_fill(form_number, test_payload = form_number) - it 'fills out a PDF from a templated JSON file' do - expected_pdf_path = "tmp/#{form_number}-tmp.pdf" + form_name = form_number.split(Regexp.union(%w[vba_ vha_]))[1].gsub('_', '-') + context "when filling the pdf for form #{form_name} given template #{test_payload}" do + it 'fills out a PDF from a templated JSON file' do + expected_pdf_path = "tmp/#{form_number}-tmp.pdf" - # remove the pdf if it already exists - FileUtils.rm_f(expected_pdf_path) + # remove the pdf if it already exists + FileUtils.rm_f(expected_pdf_path) - # fill the PDF - data = JSON.parse(File.read("modules/simple_forms_api/spec/fixtures/form_json/#{test_payload}.json")) - form = "SimpleFormsApi::#{form_number.titleize.gsub(' ', '')}".constantize.new(data) - filler = SimpleFormsApi::PdfFiller.new(form_number:, form:) - filler.generate - expect(File.exist?(expected_pdf_path)).to eq(true) + # fill the PDF + data = JSON.parse(File.read("modules/simple_forms_api/spec/fixtures/form_json/#{test_payload}.json")) + form = "SimpleFormsApi::#{form_number.titleize.gsub(' ', '')}".constantize.new(data) + filler = SimpleFormsApi::PdfFiller.new(form_number:, form:) + filler.generate + expect(File.exist?(expected_pdf_path)).to eq(true) + end end end diff --git a/modules/simple_forms_api/spec/services/pdf_stamper_spec.rb b/modules/simple_forms_api/spec/services/pdf_stamper_spec.rb index f5fdfc6bfaf..09927ed66fb 100644 --- a/modules/simple_forms_api/spec/services/pdf_stamper_spec.rb +++ b/modules/simple_forms_api/spec/services/pdf_stamper_spec.rb @@ -4,25 +4,150 @@ require SimpleFormsApi::Engine.root.join('spec', 'spec_helper.rb') describe SimpleFormsApi::PdfStamper do - def self.test_pdf_stamp_error(stamp_method, test_payload) - it 'raises an error when generating stamped file' do + let(:data) { JSON.parse(File.read("modules/simple_forms_api/spec/fixtures/form_json/#{test_payload}.json")) } + let(:form) { "SimpleFormsApi::#{test_payload.titleize.gsub(' ', '')}".constantize.new(data) } + let(:path) { 'tmp/stuff.json' } + + describe 'form-specific stamp methods' do + subject(:stamp) { described_class.send(stamp_method, generated_form_path, form) } + + before do allow(Common::FileHelpers).to receive(:random_file_path).and_return('fake/stamp_path') allow(Common::FileHelpers).to receive(:delete_file_if_exists) - allow(Prawn::Document).to receive(:generate).and_raise('Error generating stamped file') + end + + %w[21-4142 21-10210 21p-0847].each do |form_number| + context "when generating a stamped file for form #{form_number}" do + let(:stamp_method) { "stamp#{form_number.gsub('-', '')}" } + let(:test_payload) { "vba_#{form_number.gsub('-', '_')}" } + let(:generated_form_path) { 'fake/generated_form_path' } + + it 'raises an error' do + expect { stamp }.to raise_error(StandardError, /An error occurred while verifying stamp/) + end + end + end + end + + describe '.stamp107959f1' do + subject(:stamp107959f1) { described_class.stamp107959f1(path, form) } + + before do + allow(described_class).to receive(:stamp).and_return(true) + allow(File).to receive(:size).and_return(1, 2) + end + + context 'when statement_of_truth_signature is provided' do + before { stamp107959f1 } + + let(:test_payload) { 'vha_10_7959f_1' } + let(:signature) { form.data['statement_of_truth_signature'] } + let(:stamps) { [[26, 82.5, signature]] } + + it 'calls stamp with correct desired_stamp' do + expect(described_class).to have_received(:stamp).with(stamps, path, false) + end + end + end + + describe '.stamp264555' do + subject(:stamp264555) { described_class.stamp264555(path, form) } + + before do + allow(described_class).to receive(:stamp).and_return(true) + allow(File).to receive(:size).and_return(1, 2) + end + + context 'when it is called with legitimate parameters' do + before { stamp264555 } + + let(:test_payload) { 'vba_26_4555' } + let(:stamps) { [] } + + it 'calls stamp correctly' do + expect(described_class).to have_received(:stamp).with(stamps, path, false) + end + end + end + + describe '.stamp210845' do + subject(:stamp210845) { described_class.stamp210845(path, form) } - generated_form_path = 'fake/generated_form_path' - data = JSON.parse(File.read("modules/simple_forms_api/spec/fixtures/form_json/#{test_payload}.json")) - form = "SimpleFormsApi::#{test_payload.titleize.gsub(' ', '')}".constantize.new(data) + before do + allow(described_class).to receive(:multistamp).and_return(true) + allow(File).to receive(:size).and_return(1, 2) + end + + context 'when it is called with legitimate parameters' do + before { stamp210845 } - expect do - SimpleFormsApi::PdfStamper.send(stamp_method, generated_form_path, form) - end.to raise_error(RuntimeError, 'Error generating stamped file') + let(:test_payload) { 'vba_21_0845' } + let(:signature) { form.data['statement_of_truth_signature'] } + let(:page_config) do + [ + { type: :new_page }, + { type: :new_page }, + { type: :text, position: [50, 240] } + ] + end - expect(Common::FileHelpers).to have_received(:delete_file_if_exists).with('fake/stamp_path') + it 'calls multistamp correctly' do + expect(described_class).to have_received(:multistamp).with(path, signature, page_config) + end end end - test_pdf_stamp_error 'stamp214142', 'vba_21_4142' - test_pdf_stamp_error 'stamp2110210', 'vba_21_10210' - test_pdf_stamp_error 'stamp21p0847', 'vba_21p_0847' + describe '.verify' do + subject(:verify) { described_class.verify('template_path') { double } } + + before { allow(File).to receive(:size).and_return(orig_size, stamped_size) } + + describe 'when verifying a stamp' do + let(:orig_size) { 10_000 } + + context 'when the stamped file size is larger than the original' do + let(:stamped_size) { orig_size + 1 } + + it 'succeeds' do + expect { verify }.not_to raise_error + end + end + + context 'when the stamped file size is the same as the original' do + let(:stamped_size) { orig_size } + + it 'raises an error message' do + expect { verify }.to raise_error( + 'An error occurred while verifying stamp: The PDF remained unchanged upon stamping.' + ) + end + end + + context 'when the stamped file size is less than the original' do + let(:stamped_size) { orig_size - 1 } + + it 'raises an error message' do + expect { verify }.to raise_error( + 'An error occurred while verifying stamp: The PDF remained unchanged upon stamping.' + ) + end + end + end + end + + describe '.verified_multistamp' do + subject(:verified_multistamp) { described_class.verified_multistamp(path, signature_text, config) } + + before { allow(described_class).to receive(:verify).and_return(true) } + + context 'when signature_text is blank' do + let(:path) { nil } + let(:signature_text) { nil } + let(:config) { nil } + + it 'raises an error' do + expect { verified_multistamp }.to raise_error('The provided stamp content was empty.') + end + end + end end