Skip to content

Commit

Permalink
revert(vaos): reinstates url icn hashing when logging rev c59defe (#1…
Browse files Browse the repository at this point in the history
…6616)


va.gov-team#82150
  • Loading branch information
ajmagdub authored May 3, 2024
1 parent 53c5051 commit e671a2d
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ def initialize(env)
def response_values
{
detail: detail(@env.body),
source: { vamf_url: @env.url, vamf_body: @env.body, vamf_status: @env.status }
source: { vamf_url: VAOS::Anonymizers.anonymize_uri_icn(@env.url), vamf_body: @env.body,
vamf_status: @env.status }
}
end

Expand Down
21 changes: 21 additions & 0 deletions modules/vaos/app/helpers/vaos/anonymizers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,27 @@

module VAOS
module Anonymizers
# Anonymizes the ICN present in a given URI object by substituting a SHA256 digest for the ICN.
# If an ICN is not present in the URL, it would simply return the original URI.
#
# @param url [URI] URI in which ICN needs to be anonymized.
#
# @return [URI] URI with anonymized ICN (If present), original URI otherwise.
#
def self.anonymize_uri_icn(uri)
return nil if uri.nil?

# Extract the patient ICN from the URL
url = uri.to_s
match = url[/(\d{10}V\d{6})/]

return uri unless match

digest = Digest::SHA256.hexdigest(match)
url.gsub!(match, digest)
URI(url)
end

# Anonymizes the ICNs (Integration Control Number) in a given message. It scans the message for ICNs,
# which are identified by a specific pattern (\d{10}V\d{6}), and replaces each ICN with
# a SHA256 digest. If the message is nil, the method returns nil.
Expand Down
3 changes: 2 additions & 1 deletion modules/vaos/app/services/vaos/middleware/response/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ class Errors < Faraday::Middleware
def on_complete(env)
return if env.success?

Sentry.set_extras(vamf_status: env.status, vamf_body: env.response_body, vamf_url: env.url)
Sentry.set_extras(vamf_status: env.status, vamf_body: env.response_body,
vamf_url: VAOS::Anonymizers.anonymize_uri_icn(env.url))
raise VAOS::Exceptions::BackendServiceException, env
end
end
Expand Down
3 changes: 2 additions & 1 deletion modules/vaos/app/services/vaos/middleware/vaos_logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,13 @@ def call(env)
private

def log_tags(env, start_time, response_env = nil)
anon_uri = VAOS::Anonymizers.anonymize_uri_icn(env.url)
{
jti: jti(env),
status: response_env&.status,
duration: Time.current - start_time,
# service_name: service_name || 'VAOS Generic', # Need to figure out a clean way to do this with headers
url: "(#{env.method.upcase}) #{env.url}"
url: "(#{env.method.upcase}) #{anon_uri}"
}
end

Expand Down
17 changes: 17 additions & 0 deletions modules/vaos/spec/helpers/anonymizers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,23 @@
require 'rails_helper'

RSpec.describe VAOS::Anonymizers do
describe '#anonymize_uri_icn' do
it 'returns nil if the URI is nil' do
expect(subject.anonymize_uri_icn(nil)).to be_nil
end

it 'returns the original URI if the URI does not contain an ICN' do
uri = URI.parse('http://example.com')
expect(subject.anonymize_uri_icn(uri)).to be(uri)
end

it 'returns a URI with the ICN hashed' do
uri = URI.parse('http://example.com/1234567890V123456')
anon_uri = URI.parse('http://example.com/441ab560b8fc574c6bf84d6c6105318b79455321a931ef701d39f4ff91894c64')
expect(subject.anonymize_uri_icn(uri)).to eql(anon_uri)
end
end

describe '#anonymize_icns' do
let(:icn1) { '1234567890V123456' }
let(:icn2) { '0987654321V654321' }
Expand Down
6 changes: 5 additions & 1 deletion modules/vaos/spec/request/v2/appointments_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -679,9 +679,13 @@
context 'when the VAOS service errors on retrieving an appointment' do
it 'returns a 502 status code' do
VCR.use_cassette('vaos/v2/appointments/get_appointment_500', match_requests_on: %i[method path query]) do
vamf_url = 'https://veteran.apps.va.gov/vaos/v1/patients/' \
'd12672eba61b7e9bc50bb6085a0697133a5fbadf195e6cade452ddaad7921c1d/appointments/00000'
get '/vaos/v2/appointments/00000'
body = JSON.parse(response.body)
expect(response).to have_http_status(:bad_gateway)
expect(JSON.parse(response.body)['errors'][0]['code']).to eq('VAOS_502')
expect(body.dig('errors', 0, 'code')).to eq('VAOS_502')
expect(body.dig('errors', 0, 'source', 'vamf_url')).to eq(vamf_url)
end
end
end
Expand Down
11 changes: 11 additions & 0 deletions modules/vaos/spec/services/middleware/vaos_errors_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
}

let(:url) { URI.parse('url') }
let(:url_w_icn) { URI.parse('https://veteran.apps.va.gov/id/1234567890V123456') }
let(:success) { Faraday::Env.new(:get, nil, url, nil, nil, nil, nil, nil, nil, nil, 200, nil, 'response_body') }
let(:env_400) { Faraday::Env.new(:get, nil, url, nil, nil, nil, nil, nil, nil, nil, 400, nil, 'response_body') }
let(:env_403) { Faraday::Env.new(:get, nil, url, nil, nil, nil, nil, nil, nil, nil, 403, nil, 'response_body') }
Expand All @@ -27,6 +28,7 @@
let(:env_other) { Faraday::Env.new(:get, nil, url, nil, nil, nil, nil, nil, nil, nil, 600, nil, 'response_body') }
let(:env_with_error) { Faraday::Env.new(:get, nil, url, nil, nil, nil, nil, nil, nil, nil, 400, nil, JSON[error]) }
let(:env_with_errors) { Faraday::Env.new(:get, nil, url, nil, nil, nil, nil, nil, nil, nil, 400, nil, JSON[errors]) }
let(:env_w_icn) { Faraday::Env.new(:get, nil, url_w_icn, nil, nil, nil, nil, nil, nil, nil, 400, nil, JSON[errors]) }

describe 'on complete' do
context 'with success' do
Expand Down Expand Up @@ -133,6 +135,15 @@
expect(e.response_values[:detail]).not_to match('second')
}
end

it 'hashes the icn in the uri' do
expected_uri = URI('https://veteran.apps.va.gov/id/441ab560b8fc574c6bf84d6c6105318b79455321a931ef701d39f4ff91894c64')
err = VAOS::Middleware::Response::Errors.new

expect { err.on_complete(env_w_icn) }.to raise_error(VAOS::Exceptions::BackendServiceException) { |e|
expect(e.response_values.dig(:source, :vamf_url)).to eql(expected_uri)
}
end
end
end
end
18 changes: 18 additions & 0 deletions modules/vaos/spec/services/middleware/vaos_logging_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
let(:all_other_uris) { 'https://veteran.apps.va.gov/whatever' }
let(:user_service_refresh_uri) { 'https://veteran.apps.va.gov/user_service_refresh_uri' }
let(:user_service_uri) { 'https://veteran.apps.va.gov/users/v2/session?processRules=true' }
let(:appt_uri) { 'https://veteran.apps.va.gov/vaos/v1/patients/1234567890V123456/appointments' }

before do
allow(Settings.va_mobile).to receive(:key_path).and_return(fixture_file_path('open_ssl_rsa_private.pem'))
Expand Down Expand Up @@ -122,5 +123,22 @@
expect(Rails.logger).to have_received(:warn).with(rails_log_msg, anything).once
expect(StatsD).to have_received(:increment).with(statsd_msg, anything).once
end

it 'logs timeout error with hashed URI' do
expected_log_tags = {
duration: 0.0,
jti: 'unknown jti',
status: nil,
url: '(POST) https://veteran.apps.va.gov/vaos/v1/patients/' \
'441ab560b8fc574c6bf84d6c6105318b79455321a931ef701d39f4ff91894c64/appointments'
}
rails_log_msg = 'VAOS service call failed - timeout'

allow_any_instance_of(Faraday::Adapter).to receive(:call).and_raise(Faraday::TimeoutError)
allow(Rails.logger).to receive(:warn).with(rails_log_msg, anything).and_call_original

expect { client.post(appt_uri) }.to raise_error(Faraday::TimeoutError)
expect(Rails.logger).to have_received(:warn).with(rails_log_msg, expected_log_tags).once
end
end
end

0 comments on commit e671a2d

Please sign in to comment.