From 5a2debedf75038e9cbafad1f5cec885474857cb9 Mon Sep 17 00:00:00 2001 From: Jacob Penner Date: Mon, 23 Sep 2024 14:53:46 -0400 Subject: [PATCH 1/5] add logic to properly upload files to S3 --- ...teran_facing_forms_remediation_uploader.rb | 45 +++++++++++++++++++ .../s3/submission_archiver.rb | 21 +++++---- 2 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 app/uploaders/veteran_facing_forms_remediation_uploader.rb diff --git a/app/uploaders/veteran_facing_forms_remediation_uploader.rb b/app/uploaders/veteran_facing_forms_remediation_uploader.rb new file mode 100644 index 00000000000..d7b5abc503c --- /dev/null +++ b/app/uploaders/veteran_facing_forms_remediation_uploader.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +class VeteranFacingFormsRemediationUploader < CarrierWave::Uploader::Base + include SetAWSConfig + + def size_range + (1.byte)...(100_000_000.bytes) + end + + # All the same files allowed by benefits intake, with the + # addition of json for metadata and csv for manifest + def extension_allowlist + %w[bmp csv gif jpeg jpg json pdf png tif tiff txt] + end + + def initialize(benefits_intake_uuid, directory) + raise 'The benefits_intake_uuid is missing.' if @benefits_intake_uuid.blank? + + super + @benefits_intake_uuid = benefits_intake_uuid + @directory = directory + + set_storage_options! + end + + def store_dir + raise 'The s3 directory is missing.' if @directory.blank? + + @directory + end + + def set_storage_options! + # TODO: update this to vff specific S3 bucket once it has been created + s3_settings = Settings.reports.aws + # defaults to CarrierWave::Storage::File if not AWS unless a real aws_access_key_id is set + if s3_settings.aws_access_key_id.present? + set_aws_config( + s3_settings.aws_access_key_id, + s3_settings.aws_secret_access_key, + s3_settings.region, + s3_settings.bucket + ) + end + end +end diff --git a/modules/simple_forms_api/app/services/simple_forms_api/s3/submission_archiver.rb b/modules/simple_forms_api/app/services/simple_forms_api/s3/submission_archiver.rb index 5483fab907f..d6ae8762606 100644 --- a/modules/simple_forms_api/app/services/simple_forms_api/s3/submission_archiver.rb +++ b/modules/simple_forms_api/app/services/simple_forms_api/s3/submission_archiver.rb @@ -36,7 +36,7 @@ def initialize(parent_dir: 'vff-simple-forms', **options) # rubocop:disable Lint def upload log_info("Uploading archive: #{benefits_intake_uuid} to S3 bucket") - upload_temp_folder_to_s3 + upload_directory_to_s3(temp_directory_path) cleanup generate_presigned_url rescue => e @@ -72,11 +72,16 @@ def build_submission_archive(**) SubmissionArchiveBuilder.new(**).run end - def upload_temp_folder_to_s3 - Dir.glob("#{temp_directory_path}/**/*").each do |path| + def upload_directory_to_s3(directory_path) + raise "Directory #{directory_path} does not exist" unless Dir.exist?(directory_path) + + Dir.glob(File.join(directory_path, '**', '*')).each do |path| next if File.directory?(path) - File.open(path, 'rb') { |file| save_file_to_s3(file.read) } + File.open(path) do |file_obj| + sanitized_file = CarrierWave::SanitizedFile.new(file_obj) + s3_uploader.store!(sanitized_file) + end end end @@ -102,10 +107,6 @@ def generate_presigned_url submission_object.presigned_url(:get, expires_in: 30.minutes.to_i) end - def save_file_to_s3(content) - submission_object.tap { |obj| obj.put(body: content) } - end - def submission_object s3_resource.bucket(target_bucket).object(s3_submission_file_path) end @@ -131,6 +132,10 @@ def s3_directory_path @s3_directory_path ||= "#{parent_dir}/#{unique_file_name}" end + def s3_uploader + @s3_uploader ||= VeteranFacingFormsRemediationUploader.new(benefits_intake_uuid, s3_directory_path) + end + def local_submission_file_path @local_submission_file_path ||= build_local_file_path(s3_submission_file_path) end From ff2220b6fe460d1d34c1188daab0fdfdb15e0759 Mon Sep 17 00:00:00 2001 From: Jacob Penner Date: Mon, 23 Sep 2024 15:19:30 -0400 Subject: [PATCH 2/5] minor tweaks --- app/uploaders/veteran_facing_forms_remediation_uploader.rb | 1 + .../app/services/simple_forms_api/s3/submission_archiver.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/uploaders/veteran_facing_forms_remediation_uploader.rb b/app/uploaders/veteran_facing_forms_remediation_uploader.rb index d7b5abc503c..203434a62c3 100644 --- a/app/uploaders/veteran_facing_forms_remediation_uploader.rb +++ b/app/uploaders/veteran_facing_forms_remediation_uploader.rb @@ -2,6 +2,7 @@ class VeteranFacingFormsRemediationUploader < CarrierWave::Uploader::Base include SetAWSConfig + include UploaderVirusScan def size_range (1.byte)...(100_000_000.bytes) diff --git a/modules/simple_forms_api/app/services/simple_forms_api/s3/submission_archiver.rb b/modules/simple_forms_api/app/services/simple_forms_api/s3/submission_archiver.rb index d6ae8762606..363b3111758 100644 --- a/modules/simple_forms_api/app/services/simple_forms_api/s3/submission_archiver.rb +++ b/modules/simple_forms_api/app/services/simple_forms_api/s3/submission_archiver.rb @@ -133,7 +133,7 @@ def s3_directory_path end def s3_uploader - @s3_uploader ||= VeteranFacingFormsRemediationUploader.new(benefits_intake_uuid, s3_directory_path) + @s3_uploader ||= VeteranFacingFormsRemediationUploader.new(benefits_intake_uuid, s3_submission_file_path) end def local_submission_file_path From 08235f35d8d7cd38959ed837086f44111fec4243 Mon Sep 17 00:00:00 2001 From: Jacob Penner Date: Mon, 23 Sep 2024 15:57:31 -0400 Subject: [PATCH 3/5] update CODEOWNERS file --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 5f3c1446369..ab5bc2c33e7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -609,6 +609,7 @@ app/uploaders/uploader_virus_scan.rb @department-of-veterans-affairs/va-api-engi app/uploaders/validate_pdf.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/uploaders/vets_shrine.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/validators/token_util.rb @department-of-veterans-affairs/backend-review-group +app/uploaders/veteran_facing_forms_remediation_uploader.rb @department-of-veterans-affairs/platform-va-product-forms @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/account_login_statistics_job.rb @department-of-veterans-affairs/octo-identity app/sidekiq/benefits_intake_remediation_status_job.rb @department-of-veterans-affairs/platform-va-product-forms @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/benefits_intake_status_job.rb @department-of-veterans-affairs/platform-va-product-forms @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group From cff03891d3df5fb7a62778714c902d79a9e103ba Mon Sep 17 00:00:00 2001 From: Jacob Penner Date: Tue, 24 Sep 2024 10:59:53 -0400 Subject: [PATCH 4/5] add test coverage --- ...teran_facing_forms_remediation_uploader.rb | 2 +- ..._facing_forms_remediation_uploader_spec.rb | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 spec/uploaders/veteran_facing_forms_remediation_uploader_spec.rb diff --git a/app/uploaders/veteran_facing_forms_remediation_uploader.rb b/app/uploaders/veteran_facing_forms_remediation_uploader.rb index 203434a62c3..c3cf2bc7ce0 100644 --- a/app/uploaders/veteran_facing_forms_remediation_uploader.rb +++ b/app/uploaders/veteran_facing_forms_remediation_uploader.rb @@ -15,7 +15,7 @@ def extension_allowlist end def initialize(benefits_intake_uuid, directory) - raise 'The benefits_intake_uuid is missing.' if @benefits_intake_uuid.blank? + raise 'The benefits_intake_uuid is missing.' if benefits_intake_uuid.blank? super @benefits_intake_uuid = benefits_intake_uuid diff --git a/spec/uploaders/veteran_facing_forms_remediation_uploader_spec.rb b/spec/uploaders/veteran_facing_forms_remediation_uploader_spec.rb new file mode 100644 index 00000000000..4ef25eb8f01 --- /dev/null +++ b/spec/uploaders/veteran_facing_forms_remediation_uploader_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe VeteranFacingFormsRemediationUploader do + subject { described_class.new(benefits_intake_uuid, directory) } + + let(:benefits_intake_uuid) { SecureRandom.uuid } + let(:directory) { '/some/path' } + + it 'allows image, pdf, json, csv, and text files' do + expect(subject.extension_allowlist).to match_array %w[bmp csv gif jpeg jpg json pdf png tif tiff txt] + end + + it 'returns a store directory containing benefits_intake_uuid' do + expect(subject.store_dir).to eq(directory) + end + + it 'throws an error if no benefits_intake_uuid is given' do + expect { described_class.new(nil, directory) }.to raise_error(RuntimeError, 'The benefits_intake_uuid is missing.') + end + + it 'throws an error if no directory is given' do + malformed_uploader = described_class.new(benefits_intake_uuid, nil) + expect { malformed_uploader.store_dir }.to raise_error(RuntimeError, 'The s3 directory is missing.') + end +end From a3785775dde4c07bae396afecb7e358d129c5b04 Mon Sep 17 00:00:00 2001 From: Jacob Penner Date: Tue, 24 Sep 2024 11:11:39 -0400 Subject: [PATCH 5/5] update CODEOWNERS file for spec file --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index ab5bc2c33e7..2ef9b9f7772 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -2105,6 +2105,7 @@ spec/uploaders/form1010cg/poa_uploader_spec.rb @department-of-veterans-affairs/v spec/uploaders/supporting_evidence_attachment_uploader_spec.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/uploaders/uploader_virus_scan_spec.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/uploaders/validate_pdf_spec.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group +spec/uploaders/veteran_facing_forms_remediation_uploader_spec.rb @department-of-veterans-affairs/platform-va-product-forms @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/models/prescription_details.rb @department-of-veterans-affairs/vfs-mhv-medications @department-of-veterans-affairs/backend-review-group modules/my_health/app/controllers/my_health/v1/prescriptions_controller.rb @department-of-veterans-affairs/vfs-mhv-medications @department-of-veterans-affairs/backend-review-group modules/my_health/spec/request/v1/prescriptions_request_spec.rb @department-of-veterans-affairs/vfs-mhv-medications @department-of-veterans-affairs/backend-review-group