Skip to content

Commit

Permalink
Merge pull request #751 from DFE-Digital/112-malware-scanning-of-file…
Browse files Browse the repository at this point in the history
…-uploads

112 malware scanning of file uploads
  • Loading branch information
richardpattinson authored Jul 23, 2024
2 parents 9d0e044 + 32ff923 commit f992276
Show file tree
Hide file tree
Showing 17 changed files with 319 additions and 1 deletion.
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,8 @@ GEM
nio4r (2.7.3)
nokogiri (1.16.6-arm64-darwin)
racc (~> 1.4)
nokogiri (1.16.6-x86_64-darwin)
racc (~> 1.4)
nokogiri (1.16.6-x86_64-linux)
racc (~> 1.4)
notifications-ruby-client (6.0.0)
Expand Down Expand Up @@ -645,6 +647,7 @@ PLATFORMS
arm64-darwin-22
arm64-darwin-23
arm64-darwin-24
x86_64-darwin-23
x86_64-linux

DEPENDENCIES
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def confirm
date_of_birth_change: @date_of_birth_change
)
@date_of_birth_change.update!(reference_number:)
@date_of_birth_change.malware_scan

redirect_to submitted_qualifications_one_login_user_date_of_birth_change_path(@date_of_birth_change)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def confirm
@name_change = current_user.name_changes.find(params[:id])
reference_number = qualifications_api_client.send_name_change(name_change: @name_change)
@name_change.update!(reference_number:)
@name_change.malware_scan

redirect_to submitted_qualifications_one_login_user_name_change_path(@name_change)
end
Expand Down
9 changes: 9 additions & 0 deletions app/jobs/fetch_malware_scan_result_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

class FetchMalwareScanResultJob < ApplicationJob
discard_on ActiveRecord::RecordNotFound

def perform(change_request:)
Malware::FetchScanResult.new(change_request:).call
end
end
7 changes: 7 additions & 0 deletions app/jobs/remove_malware_file_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class RemoveMalwareFileJob < ApplicationJob
def perform(change_request:)
Malware::RemoveFile.new(change_request:).call
end
end
14 changes: 14 additions & 0 deletions app/models/date_of_birth_change.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,21 @@ class DateOfBirthChange < ApplicationRecord

delegate :filename, to: :evidence, prefix: true

enum malware_scan_result: {
clean: "clean",
error: "error",
pending: "pending",
suspect: "suspect"
}, _prefix: :scan_result


def expiring_evidence_url
evidence.url(expires_in: 5.minutes.in_seconds)
end

def malware_scan
return unless evidence.attached?

FetchMalwareScanResultJob.set(wait: 30.seconds).perform_later(change_request: self)
end
end
56 changes: 56 additions & 0 deletions app/models/malware/fetch_scan_result.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

module Malware
class FetchScanResult
# Only later versions of the Azure Storage REST API support tags operations.
Kernel.silence_warnings { Azure::Storage::Blob::Default::STG_VERSION = "2022-11-02" }

SCAN_RESULT_TAG_KEY = /Malware Scanning scan result/
SCAN_RESULT_TAG_VALUE_CLEAN = /No threats found/

def initialize(change_request:)
@change_request = change_request
end

def call
return unless change_request.evidence.attached?

response = fetch_scan_result
if response.success?
if response.body =~ SCAN_RESULT_TAG_KEY
change_request.update!(malware_scan_result: malware_scan_result_from_response(response))
RemoveMalwareFileJob.perform_later(change_request:) if change_request.scan_result_suspect?
end
else
change_request.scan_result_error!
end
rescue Azure::Core::Http::HTTPError
change_request.scan_result_error!
end

private

attr_reader :change_request

def blob_service
@blob_service ||=
Azure::Storage::Blob::BlobService.new(
storage_account_name: ENV["AZURE_STORAGE_ACCOUNT_NAME"],
storage_access_key: ENV["AZURE_STORAGE_ACCESS_KEY"]
)
end

def fetch_scan_result
blob_service.call(:get, get_tags_for_blob_url)
end

def get_tags_for_blob_url
@get_tags_for_blob_url ||=
blob_service.generate_uri(File.join("evidence", change_request.evidence.key), { comp: "tags" })
end

def malware_scan_result_from_response(response)
response.body =~ SCAN_RESULT_TAG_VALUE_CLEAN ? "clean" : "suspect"
end
end
end
17 changes: 17 additions & 0 deletions app/models/malware/remove_file.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true

module Malware
class RemoveFile
attr_reader :change_request

def initialize(change_request:)
@change_request = change_request
end

def call
return unless change_request.scan_result_suspect?

change_request.evidence.purge
end
end
end
15 changes: 15 additions & 0 deletions app/models/name_change.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ class NameChange < ApplicationRecord
belongs_to :user
has_one_attached :evidence

enum malware_scan_result: {
clean: "clean",
error: "error",
pending: "pending",
suspect: "suspect"
},
_prefix: :scan_result


def expiring_evidence_url
evidence.url(expires_in: 5.minutes.in_seconds)
end
Expand All @@ -11,4 +20,10 @@ def expiring_evidence_url
def full_name
"#{first_name} #{middle_name} #{last_name}".squish
end

def malware_scan
return unless evidence.attached?

FetchMalwareScanResultJob.set(wait: 30.seconds).perform_later(change_request: self)
end
end
2 changes: 2 additions & 0 deletions config/analytics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ shared:
- created_at
- updated_at
- reference_number
- malware_scan_result
:dsi_users:
- id
- email
Expand Down Expand Up @@ -62,6 +63,7 @@ shared:
- middle_name
- last_name
- reference_number
- malware_scan_result
:users:
- id
- email
Expand Down
3 changes: 3 additions & 0 deletions config/feature_flags.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,6 @@ feature_flags:
author: Malcolm Baig
description: Reduce number of search results shown to users of CTR by prompting
them for a TRN when last name and date of birth return multiple results.
malware_scan:
author: Richard Pattinson
description: Enable malware scanning
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddMalwareScanResultToChangeRequests < ActiveRecord::Migration[7.1]
def change
add_column :date_of_birth_changes, :malware_scan_result, :string, null: false, default: "pending"
add_column :name_changes, :malware_scan_result, :string, null: false, default: "pending"
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2024_07_10_162148) do
ActiveRecord::Schema[7.1].define(version: 2024_07_17_113527) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

Expand Down Expand Up @@ -96,6 +96,7 @@
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "reference_number"
t.string "malware_scan_result", default: "pending", null: false
t.index ["user_id"], name: "index_date_of_birth_changes_on_user_id"
end

Expand Down Expand Up @@ -148,6 +149,7 @@
t.string "middle_name"
t.string "last_name"
t.string "reference_number"
t.string "malware_scan_result", default: "pending", null: false
t.index ["user_id"], name: "index_name_changes_on_user_id"
end

Expand Down
19 changes: 19 additions & 0 deletions spec/support/system/malware_scan_helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module MalwareScanHelpers
def given_malware_scanning_is_enabled(scan_result: "No threats found")
FeatureFlags::FeatureFlag.activate(:malware_scan)
tags_url = "https://example.com/uploads/abc987xyz123?comp=tags"
response_body = <<-XML.squish
<Tags>
<Tag>
<Key>Malware Scanning scan result</Key>
<Value>#{scan_result}</Value>
</Tag>
</Tags>
XML
stubbed_service = instance_double(Azure::Storage::Blob::BlobService, generate_uri: tags_url)
stubbed_response =
instance_double(Azure::Core::Http::HttpResponse, success?: true, body: response_body)
allow(Azure::Storage::Blob::BlobService).to receive(:new).and_return(stubbed_service)
allow(stubbed_service).to receive(:call).and_return(stubbed_response)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
RSpec.feature "Account page", type: :system do
include CommonSteps
include QualificationAuthenticationSteps
include MalwareScanHelpers

scenario "User submits a request to change date of birth", test: %i[with_stubbed_auth with_fake_quals_api] do
given_the_qualifications_service_is_open
given_i_am_signed_in_via_one_login
given_malware_scanning_is_enabled
when_i_click_through_to_update_my_details

and_click_change_date_of_birth
Expand All @@ -27,6 +29,9 @@

then_my_request_is_submitted
and_my_evidence_is_uploaded

when_the_pending_scan_result_is_fetched_after_a_delay
then_the_evidence_is_updated
end

private
Expand Down Expand Up @@ -102,5 +107,15 @@ def then_my_request_is_submitted

def and_my_evidence_is_uploaded
expect(DateOfBirthChange.last.evidence.attached?).to be true
expect(DateOfBirthChange.last.malware_scan_result).to eq "pending"
end

def when_the_pending_scan_result_is_fetched_after_a_delay
time = Time.zone.now + 1.minute
travel_to(time) { perform_enqueued_jobs(only: FetchMalwareScanResultJob) }
end

def then_the_evidence_is_updated
expect(DateOfBirthChange.last.malware_scan_result).to eq "clean"
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
RSpec.feature "Account page", type: :system do
include CommonSteps
include QualificationAuthenticationSteps
include MalwareScanHelpers

scenario "User submits a request to change name", test: %i[with_stubbed_auth with_fake_quals_api] do
given_the_qualifications_service_is_open
given_i_am_signed_in_via_one_login
given_malware_scanning_is_enabled
when_i_click_through_to_update_my_details
and_click_change_name
then_i_am_on_the_name_change_form
Expand All @@ -25,6 +27,9 @@

then_my_request_is_submitted
and_my_evidence_is_uploaded

when_the_pending_scan_result_is_fetched_after_a_delay
then_the_evidence_is_updated
end

private
Expand Down Expand Up @@ -103,5 +108,15 @@ def then_my_request_is_submitted

def and_my_evidence_is_uploaded
expect(NameChange.last.evidence.attached?).to be true
expect(NameChange.last.malware_scan_result).to eq "pending"
end

def when_the_pending_scan_result_is_fetched_after_a_delay
time = Time.zone.now + 1.minute
travel_to(time) { perform_enqueued_jobs(only: FetchMalwareScanResultJob) }
end

def then_the_evidence_is_updated
expect(NameChange.last.malware_scan_result).to eq "clean"
end
end
Loading

0 comments on commit f992276

Please sign in to comment.