Skip to content

Commit

Permalink
Merge pull request #605 from communitiesuk/ratonvirus-scanning-consen…
Browse files Browse the repository at this point in the history
…t-forms

Scan uploaded consent forms with ratonvirus
  • Loading branch information
monotypical authored Jan 29, 2025
2 parents 937d327 + 391416f commit a71af6e
Show file tree
Hide file tree
Showing 15 changed files with 144 additions and 35 deletions.
6 changes: 4 additions & 2 deletions app/controllers/unaccompanied_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,12 @@ def handle_upload_uk
@application.uk_parental_consent_filename = upload_params.original_filename
@application.uk_parental_consent_saved_filename = "#{SecureRandom.uuid.upcase}-#{upload_params.original_filename}"
@application.uk_parental_consent_file_size = upload_params.size
@application.uk_parental_consent_tempfile_path = upload_params.path
rescue ActionController::ParameterMissing
# Do nothing!
Rails.logger.debug "No upload file found!"
end
@application.partial_validation = %i[uk_parental_consent_file_type uk_parental_consent_filename uk_parental_consent_saved_filename uk_parental_consent_file_size]
@application.partial_validation = %i[uk_parental_consent_file_type uk_parental_consent_filename uk_parental_consent_saved_filename uk_parental_consent_file_size uk_parental_consent_tempfile_path]
if @application.valid?
save_and_redirect(@application.uk_parental_consent_saved_filename, upload_params.tempfile)
else
Expand All @@ -155,11 +156,12 @@ def handle_upload_ukraine
@application.ukraine_parental_consent_filename = upload_params.original_filename
@application.ukraine_parental_consent_saved_filename = "#{SecureRandom.uuid.upcase}-#{upload_params.original_filename}"
@application.ukraine_parental_consent_file_size = upload_params.size
@application.ukraine_parental_consent_tempfile_path = upload_params.path
rescue ActionController::ParameterMissing
# Do nothing!
Rails.logger.debug "No upload file found!"
end
@application.partial_validation = %i[ukraine_parental_consent_file_type ukraine_parental_consent_filename ukraine_parental_consent_saved_filename ukraine_parental_consent_file_size]
@application.partial_validation = %i[ukraine_parental_consent_file_type ukraine_parental_consent_filename ukraine_parental_consent_saved_filename ukraine_parental_consent_file_size ukraine_parental_consent_tempfile_path]
if @application.valid?
save_and_redirect(@application.ukraine_parental_consent_saved_filename, upload_params.tempfile)
else
Expand Down
4 changes: 4 additions & 0 deletions app/models/concerns/common_validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,8 @@ def uk_mobile_number?(value)
end
phone_number_valid?(value) && value.present? && blanks_removed.match?(valid_uk_number)
end

def file_not_malicious?(filepath)
!Ratonvirus.scanner.virus?(filepath)
end
end
14 changes: 14 additions & 0 deletions app/models/concerns/uam_validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,13 @@ module UamValidations
validate :validate_sponsor_date_of_birth, if: -> { run_validation? :sponsor_date_of_birth }
validate :validate_sponsor_declaration, if: -> { run_validation? :sponsor_declaration }
validate :validate_uk_parent_consent_filename, if: -> { run_validation? :uk_parental_consent_filename }
validate :validate_uk_parental_consent_tempfile_path, if: -> { run_validation? :uk_parental_consent_tempfile_path }
validate :validate_uk_parent_consent_file_size, if: -> { run_validation? :uk_parental_consent_file_size }
validate :validate_uk_parent_consent_file_type, if: -> { run_validation? :uk_parental_consent_file_type }
validate :validate_ukraine_parent_consent_filename, if: -> { run_validation? :ukraine_parental_consent_filename }
validate :validate_ukraine_parent_consent_file_size, if: -> { run_validation? :ukraine_parental_consent_file_size }
validate :validate_ukraine_parent_consent_file_type, if: -> { run_validation? :ukraine_parental_consent_file_type }
validate :validate_ukraine_parental_consent_tempfile_path, if: -> { run_validation? :ukraine_parental_consent_tempfile_path }
end

def nationality_permitted_value(nationality)
Expand Down Expand Up @@ -197,6 +199,12 @@ def validate_uk_parent_consent_filename
end
end

def validate_uk_parental_consent_tempfile_path
if @uk_parental_consent_tempfile_path.present? && !file_not_malicious?(@uk_parental_consent_tempfile_path)
errors.add(:uk_parental_consent, I18n.t(:file_malicious, scope: :error))
end
end

def validate_uk_parent_consent_file_size
if !@uk_parental_consent_file_size.nil? && @uk_parental_consent_file_size > 1024 * 1024 * 20
errors.add(:uk_parental_consent, I18n.t(:file_too_large, scope: :error))
Expand All @@ -215,6 +223,12 @@ def validate_ukraine_parent_consent_filename
end
end

def validate_ukraine_parental_consent_tempfile_path
if @ukraine_parental_consent_tempfile_path.present? && !file_not_malicious?(@ukraine_parental_consent_tempfile_path)
errors.add(:ukraine_parental_consent, I18n.t(:file_malicious, scope: :error))
end
end

def validate_ukraine_parent_consent_file_size
if !@ukraine_parental_consent_file_size.nil? && @ukraine_parental_consent_file_size > 1024 * 1024 * 20
errors.add(:ukraine_parental_consent, I18n.t(:file_too_large, scope: :error))
Expand Down
7 changes: 2 additions & 5 deletions app/models/unaccompanied_minor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,18 @@ class UnaccompaniedMinor < ApplicationRecord
:is_permitted,
:have_parental_consent,
:have_parental_consent_options,
:parental_consent,
:uk_parental_consent_file_type,
:uk_parental_consent_filename,
:uk_parental_consent_saved_filename,
:uk_parental_consent_file_size,
:uk_parental_consent_tempfile_path,
:uk_parental_consent_file_upload_rid,
:uk_parental_consent_file_uploaded_timestamp,
:ukraine_parental_consent_file_type,
:ukraine_parental_consent_filename,
:ukraine_parental_consent_saved_filename,
:ukraine_parental_consent_file_size,
:ukraine_parental_consent_tempfile_path,
:ukraine_parental_consent_file_upload_rid,
:ukraine_parental_consent_file_uploaded_timestamp,
:minor_date_of_birth,
Expand Down Expand Up @@ -113,10 +114,6 @@ class UnaccompaniedMinor < ApplicationRecord
assign_attributes(answers)
end

has_one_attached :parental_consent

validates :parental_consent, antivirus: true # Add this for antivirus validation

def formatted_address?
[@residential_line_1, @residential_line_2, @residential_town, @residential_postcode].reject(&:blank?).join(", ")
end
Expand Down
1 change: 0 additions & 1 deletion config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
require "active_model/railtie"
require "active_job/railtie"
require "active_record/railtie"
# require "active_storage/engine"
require "action_controller/railtie"
require "action_mailer/railtie"
require "action_mailbox/engine"
Expand Down
3 changes: 0 additions & 3 deletions config/environments/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@
config.cache_store = :null_store
end

# Store uploaded files on the local file system (see config/storage.yml for options)
config.active_storage.service = :local

# Don't care if the mailer can't send.
config.action_mailer.raise_delivery_errors = false

Expand Down
3 changes: 0 additions & 3 deletions config/environments/production.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@
# config.action_dispatch.x_sendfile_header = 'X-Sendfile' # for Apache
# config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX

# Store uploaded files on the local file system (see config/storage.yml for options)
config.active_storage.service = :amazon

# Mount Action Cable outside main process or domain
# config.action_cable.mount_path = nil
# config.action_cable.url = 'wss://example.com/cable'
Expand Down
3 changes: 0 additions & 3 deletions config/environments/staging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@
# config.action_dispatch.x_sendfile_header = 'X-Sendfile' # for Apache
# config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX

# Store uploaded files on the local file system (see config/storage.yml for options)
config.active_storage.service = :amazon

# Mount Action Cable outside main process or domain
# config.action_cable.mount_path = nil
# config.action_cable.url = 'wss://example.com/cable'
Expand Down
3 changes: 0 additions & 3 deletions config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@
# Disable request forgery protection in test environment.
config.action_controller.allow_forgery_protection = false

# Store uploaded files on the local file system in a temporary directory
config.active_storage.service = :test

config.action_mailer.perform_caching = false

# Tell Action Mailer not to deliver emails to the real world.
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/ratonvirus.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# config/initializers/ratonvirus.rb
Ratonvirus.configure do |config|
config.scanner = :eicar
config.storage = :active_storage
config.storage = :filepath
end
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ en:
invalid_file_type_chosen: You can only upload pdf, jpeg or png files
no_file_chosen: You must choose a file
file_too_large: Your file must be smaller than 20MB
file_malicious: The uploaded file has been detected as malicious. Please upload a different file
invalid_date_of_birth: Enter a valid date of birth
date_of_birth_future: This date cannot be in the future. Enter a valid date of birth.
too_old_date_of_birth: They must be under 18 to be considered a child in the UK
Expand Down
14 changes: 0 additions & 14 deletions config/storage.yml

This file was deleted.

63 changes: 63 additions & 0 deletions spec/models/unaccompanied_minor_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "rails_helper"
require "tempfile"

RSpec.describe UnaccompaniedMinor, type: :model do
let(:given_name_error) { "You must enter a valid given name" }
Expand Down Expand Up @@ -271,6 +272,68 @@
app.have_parental_consent = "yes"
expect(app.valid?).to be(true)
end

it "allows non-malicious UK consent files" do
app = described_class.new
app.partial_validation = [:uk_parental_consent_tempfile_path]

file = Tempfile.new
begin
file.write("This is an example of non-malicious file contents")
app.uk_parental_consent_tempfile_path = file.path
expect(app.valid?).to be(true)
ensure
file.close
file.unlink
end
end

it "rejects malicious UK consent files" do
app = described_class.new
app.partial_validation = [:uk_parental_consent_tempfile_path]

begin
file = make_malicious_file
app.uk_parental_consent_tempfile_path = file.path

expect(app.valid?).to be(false)
expect(app.errors[:uk_parental_consent]).to include("The uploaded file has been detected as malicious. Please upload a different file")
ensure
file.close
file.unlink
end
end

it "allows non-malicious Ukraine consent files" do
app = described_class.new
app.partial_validation = [:ukraine_parental_consent_tempfile_path]

file = Tempfile.new
begin
file.write("This is an example of non-malicious file contents")
app.ukraine_parental_consent_tempfile_path = file.path
expect(app.valid?).to be(true)
ensure
file.close
file.unlink
end
end

it "rejects malicious Ukraine consent files" do
app = described_class.new
app.partial_validation = [:ukraine_parental_consent_tempfile_path]

begin
file = make_malicious_file
app.ukraine_parental_consent_tempfile_path = file.path

expect(app.valid?).to be(false)
expect(app.errors[:ukraine_parental_consent]).to include("The uploaded file has been detected as malicious. Please upload a different file")
ensure
file.close
file.unlink
end
end
end

describe "age validations" do
Expand Down
16 changes: 16 additions & 0 deletions spec/support/unaccompanied_minor_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require "tempfile"

module UnaccompaniedMinorHelpers
START_PAGE_CONTENT = "You are eligible to use this service".freeze
TASK_LIST_CONTENT = "Apply for approval to provide a safe home for a child from Ukraine".freeze
Expand Down Expand Up @@ -322,4 +324,18 @@ def uam_enter_minors_contact_details(email: nil, confirm_email: nil, telephone:
click_on("Continue")
end
end

def make_malicious_file
file = Tempfile.new(["malicious-test-file", ".pdf"])
# We need to construct the EICAR test string from multiple parts because if it appears in it's entirely in the
# source file our dev machine's AV will be unhappy

# rubocop:disable Style/StringConcatenation
file.write("X5O!P%@AP[4\\PZX54(P^)7CC)7}$EICAR" + "-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*")
# rubocop:enable Style/StringConcatenation

file.close

file
end
end
39 changes: 39 additions & 0 deletions spec/system/unaccompanied_minor_file_upload_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
RSpec.describe "Unaccompanied minor expression of interest", type: :system do
let(:task_list_content) { "Apply for approval to provide a safe home for a child from Ukraine".freeze }
let(:malicious_file_message) { "The uploaded file has been detected as malicious. Please upload a different file".freeze }

before do
driven_by(:rack_test_user_agent)
Expand Down Expand Up @@ -66,5 +67,43 @@

expect(page).to have_content("Your file must be smaller than 20MB")
end

it "gets rejected trying to upload a malicious UK consent form" do
expect(page).to have_content(task_list_content)
click_link("Upload UK consent form")
expect(page).to have_content("You must upload 2 completed parental consent forms")
click_button("Continue")

expect(page).to have_content("Upload the UK sponsorship arrangement consent form")

begin
malicious_file = make_malicious_file
attach_file("unaccompanied-minor-uk-parental-consent-field", malicious_file.path)
click_button("Continue")

expect(page).to have_content(malicious_file_message)
ensure
malicious_file.close
malicious_file.unlink
end
end

it "gets rejected trying to upload a malicious Ukraine consent form" do
expect(page).to have_content(task_list_content)
click_link("Upload Ukrainian consent form")

expect(page).to have_content("Upload the Ukraine certified consent form")

begin
malicious_file = make_malicious_file
attach_file("unaccompanied-minor-ukraine-parental-consent-field", malicious_file.path)
click_button("Continue")

expect(page).to have_content(malicious_file_message)
ensure
malicious_file.close
malicious_file.unlink
end
end
end
end

0 comments on commit a71af6e

Please sign in to comment.