Skip to content

Commit

Permalink
Added file extension type restrictions and updated specs
Browse files Browse the repository at this point in the history
  • Loading branch information
danlim715 committed Dec 4, 2024
1 parent bc56c8b commit 39fff39
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 36 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -2114,6 +2114,7 @@ spec/support/vcr_cassettes/spec/support @department-of-veterans-affairs/octo-ide
spec/support/vcr_cassettes/staccato @department-of-veterans-affairs/vfs-10-10 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/support/vcr_cassettes/token_validation @department-of-veterans-affairs/lighthouse-banana-peels @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/support/vcr_cassettes/travel_pay @department-of-veterans-affairs/travel-pay-integration @department-of-veterans-affairs/backend-review-group
spec/support/vcr_cassettes/uploads/validate_document.yml @department-of-veterans-affairs/pension-and-burials @department-of-veterans-affairs/backend-review-group
spec/spupport/vcr_cassettes/user/get_facilities_empty.yml @department-of-veterans-affairs/vfs-facilities-frontend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/support/vcr_cassettes/va_forms @department-of-veterans-affairs/platform-va-product-forms @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/support/vcr_cassettes/va_notify @department-of-veterans-affairs/va-notify-write @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
Expand Down
44 changes: 27 additions & 17 deletions app/controllers/v0/claim_documents_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,23 @@ class ClaimDocumentsController < ApplicationController
def create
uploads_monitor.track_document_upload_attempt(form_id, current_user)

attachment = klass.new(form_id:)
@attachment = klass.new(form_id:)
# add the file after so that we have a form_id and guid for the uploader to use
attachment.file = unlock_file(params['file'], params['password'])
@attachment.file = unlock_file(params['file'], params['password'])

if Flipper.enabled?(:document_upload_validation_enabled) && !stamped_pdf_valid?(attachment)
raise Common::Exceptions::ValidationErrors, attachment
if Flipper.enabled?(:document_upload_validation_enabled) && !stamped_pdf_valid?
raise Common::Exceptions::ValidationErrors, @attachment
end

raise Common::Exceptions::ValidationErrors, attachment unless attachment.valid?
raise Common::Exceptions::ValidationErrors, @attachment unless @attachment.valid?

attachment.save
@attachment.save

uploads_monitor.track_document_upload_success(form_id, attachment.id, current_user)
uploads_monitor.track_document_upload_success(form_id, @attachment.id, current_user)

render json: PersistentAttachmentSerializer.new(attachment)
render json: PersistentAttachmentSerializer.new(@attachment)
rescue => e
uploads_monitor.track_document_upload_failed(form_id, attachment&.id, current_user, e)
uploads_monitor.track_document_upload_failed(form_id, @attachment&.id, current_user, e)
raise e
end

Expand Down Expand Up @@ -78,15 +78,25 @@ def unlock_file(file, file_password)
file
end

def stamped_pdf_valid?(attachment)
document = PDFUtilities::DatestampPdf.new(attachment.to_pdf).run(text: 'VA.GOV', x: 5, y: 5)
document = PDFUtilities::DatestampPdf.new(document).run(
text: 'FDC Reviewed - VA.gov Submission',
x: 429,
y: 770,
text_only: true
)
def stamped_pdf_valid?
extension = File.extname(@attachment&.file&.id)
allowed_types = PersistentAttachment::ALLOWED_DOCUMENT_TYPES

unless allowed_types.include?(extension)
raise Common::Exceptions::UnprocessableEntity.new(
detail: I18n.t('errors.messages.extension_allowlist_error', extension:, allowed_types:),
source: 'PersistentAttachment.unlock_file'
)
end

document = PDFUtilities::DatestampPdf.new(@attachment.to_pdf).run(text: 'VA.GOV', x: 5, y: 5)
intake_service.valid_document?(document:)
rescue BenefitsIntake::Service::InvalidDocumentError => e
@attachment.errors.add(:attachment, e.message)
false
rescue PdfForms::PdftkError => e

Check failure on line 97 in app/controllers/v0/claim_documents_controller.rb

View workflow job for this annotation

GitHub Actions / Linting and Security

Lint/UselessAssignment: Useless assignment to variable - `e`.
@attachment.errors.add(:attachment, 'File is corrupt and cannot be uploaded')
false
end

def intake_service
Expand Down
2 changes: 2 additions & 0 deletions app/models/persistent_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
class PersistentAttachment < ApplicationRecord
include SetGuid

ALLOWED_DOCUMENT_TYPES = %w[.pdf .jpg .jpeg .png].freeze

has_kms_key
has_encrypted :file_data, key: :kms_key, **lockbox_options
belongs_to :saved_claim, inverse_of: :persistent_attachments, optional: true
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/v0/claim_documents_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

RSpec.describe V0::ClaimDocumentsController, type: :controller do
let(:user) { create(:user) }
let(:file) { fixture_file_upload('doctors-note.gif') }
let(:file) { fixture_file_upload('doctors-note.jpg') }
let(:password) { 'password' }
let(:params) { { form_id: '21P-527EZ', file: file, password: password } }
let(:attachment) { build(:persistent_attachment_va_form, file_data: file.to_json) }
Expand Down
38 changes: 20 additions & 18 deletions spec/requests/swagger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -297,25 +297,27 @@
end

it 'supports adding a claim document' do
expect(subject).to validate(
:post,
'/v0/claim_attachments',
200,
'_data' => {
'form_id' => '21P-530V2',
file: fixture_file_upload('spec/fixtures/files/doctors-note.pdf')
}
)
VCR.use_cassette('uploads/validate_document') do
expect(subject).to validate(
:post,
'/v0/claim_attachments',
200,
'_data' => {
'form_id' => '21P-530V2',
file: fixture_file_upload('spec/fixtures/files/doctors-note.pdf')
}
)

expect(subject).to validate(
:post,
'/v0/claim_attachments',
422,
'_data' => {
'form_id' => '21P-530V2',
file: fixture_file_upload('spec/fixtures/files/empty_file.txt')
}
)
expect(subject).to validate(
:post,
'/v0/claim_attachments',
422,
'_data' => {
'form_id' => '21P-530V2',
file: fixture_file_upload('spec/fixtures/files/empty_file.txt')
}
)
end
end

it 'supports checking stem_claim_status' do
Expand Down
Loading

0 comments on commit 39fff39

Please sign in to comment.