Skip to content

Commit

Permalink
[CST] Ensure that EVSS doc upload files have extensions that matches …
Browse files Browse the repository at this point in the history
…their contents (#16498)

* Ensuring that EVSS Document upload files have content that matches their extension

* Don't run new model validation if there is no file_obj to run it against

* Updating CODEOWNERS
  • Loading branch information
jerekshoe committed Apr 26, 2024
1 parent 52de5b4 commit 22d0fd9
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 6 deletions.
8 changes: 4 additions & 4 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Expand Up @@ -224,9 +224,9 @@ app/models/education_benefits_claim.rb @department-of-veterans-affairs/my-educat
app/models/education_benefits_submission.rb @department-of-veterans-affairs/my-education-benefits @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/education_stem_automated_decision.rb @department-of-veterans-affairs/Benefits-Team-1 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/eligible_data_class.rb @department-of-veterans-affairs/vfs-vaos @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/evss_claim_document.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/evss_claim_document.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/evss_claim.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/evss_claims_sync_status_tracker.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/evss_claims_sync_status_tracker.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/expiry_scanner.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/external_services_redis/status.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/facilities @department-of-veterans-affairs/vfs-facilities-frontend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
Expand Down Expand Up @@ -1252,7 +1252,7 @@ spec/sidekiq/evss/disability_compensation_form/submit_form526_spec.rb @departmen
spec/sidekiq/evss/disability_compensation_form/submit_form8940_spec.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/sidekiq/evss/disability_compensation_form/submit_uploads_spec.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/sidekiq/evss/disability_compensation_form/upload_bdd_instructions_spec.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/sidekiq/evss/document_upload_spec.rb @department-of-veterans-affairs/backend-review-group
spec/sidekiq/evss/document_upload_spec.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/sidekiq/evss/retrieve_claims_from_remote_job_spec.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/sidekiq/evss/update_claim_from_remote_job_spec.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/sidekiq/export_breaker_status_spec.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
Expand Down Expand Up @@ -1535,7 +1535,7 @@ spec/requests/csrf_request_spec.rb @department-of-veterans-affairs/octo-identity
spec/requests/debts_spec.rb @department-of-veterans-affairs/vsa-debt-resolution @department-of-veterans-affairs/backend-review-group
spec/requests/decision_review_evidences_request_spec.rb @department-of-veterans-affairs/Benefits-Team-1 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/requests/disability_compensation_form_request_spec.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/requests/documents_spec.rb @department-of-veterans-affairs/Benefits-Team-1 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/requests/documents_spec.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/requests/education_benefits_claims_request_spec.rb @department-of-veterans-affairs/my-education-benefits @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/requests/evss_claims_async_spec.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/requests/evss_claims_spec.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
Expand Down
22 changes: 22 additions & 0 deletions app/models/evss_claim_document.rb
Expand Up @@ -18,6 +18,7 @@ class EVSSClaimDocument < Common::Base

validates(:file_name, presence: true)
validate :known_document_type?
validate :content_type_matches_extension?
validate :unencrypted_pdf?
before_validation :normalize_text, :convert_to_unlocked_pdf, :normalize_file_name

Expand Down Expand Up @@ -81,6 +82,27 @@ def tracked_item_id=(num)

private

def content_type_matches_extension?
return unless file_obj

true_mime_type = MimeMagic.by_magic(File.open(file_obj.tempfile.path)).to_s

# MimeMagic cannot always determine the mime_type and will sometimes
# return ''. In those cases it makes sense to fall back to the content_type
# as passed in when the request is made
true_mime_type = file_obj.content_type if true_mime_type.empty?

assumed_mime_type = MimeMagic.by_extension(extension).to_s

errors.add(:base, I18n.t('errors.messages.uploads.content_type_mismatch')) if true_mime_type != assumed_mime_type
end

def extension
# Using file_name instead of file_path because the temp path doesn't include
# an extension
File.extname(file_name).downcase[1..] # Remove the leading dot
end

def known_document_type?
errors.add(:base, I18n.t('errors.messages.uploads.document_type_unknown')) unless description
end
Expand Down
3 changes: 2 additions & 1 deletion config/locales/en.yml
Expand Up @@ -53,7 +53,8 @@ en:
# end carrierwave messages

uploads:
document_type_unknown: 'We couldn’t upload your file because it’s not a known document type.'
content_type_mismatch: 'We couldn’t upload your file because the content doesn’t match the extension.'
document_type_unknown: 'We couldn’t upload your file because it’s not a known document type.'
encrypted: 'We couldn’t upload your PDF because it’s encrypted. Save your file without a password and try uploading it again'
malformed_pdf: 'We couldn’t upload your PDF because the file is corrupted'
ascii_encoded: "We couldn’t upload your file because we can’t read its characters. Text files must be ASCII-encoded."
Expand Down
1 change: 1 addition & 0 deletions spec/fixtures/files/html.txt
@@ -0,0 +1 @@
<html><head></head><body>Fake TXT file</body></html>
15 changes: 14 additions & 1 deletion spec/requests/documents_spec.rb
Expand Up @@ -42,6 +42,19 @@
expect(args['tracked_item_id']).to be_nil
end

context 'when content type and file extension don’t match' do
let(:file) { fixture_file_upload('html.txt', 'text/plain') }

it 'rejects files when the content type and extension don’t match' do
params = { file:, tracked_item_id:, document_type: }
post('/v0/evss_claims/189625/documents', params:)
expect(response.status).to eq(422)
expect(JSON.parse(response.body)['errors'].first['title']).to eq(
I18n.t('errors.messages.uploads.content_type_mismatch')
)
end
end

context 'with unaccepted file_type' do
let(:file) { fixture_file_upload('invalid_idme_cert.crt', 'application/x-x509-ca-cert') }

Expand Down Expand Up @@ -70,7 +83,7 @@
expect(JSON.parse(response.body)['job_id']).to eq(EVSS::DocumentUpload.jobs.first['jid'])
end

it 'rejects locked PDFs with the incocorrect password' do
it 'rejects locked PDFs with the incorrect password' do
params = { file: locked_file, tracked_item_id:, document_type:, password: 'bad' }
post('/v0/evss_claims/189625/documents', params:)

Expand Down

0 comments on commit 22d0fd9

Please sign in to comment.