From adf8a662bc7bfb1c22fcc11dbbba9267d8a99e3d Mon Sep 17 00:00:00 2001 From: Eric Tillberg Date: Wed, 17 Apr 2024 13:38:08 -0400 Subject: [PATCH 1/7] Refactor uploads spec, simple forms (#16380) * Refactor uploads spec, simple forms * rubocop --- .../spec/requests/v1/uploads_spec.rb | 462 +++++++++--------- 1 file changed, 238 insertions(+), 224 deletions(-) diff --git a/modules/simple_forms_api/spec/requests/v1/uploads_spec.rb b/modules/simple_forms_api/spec/requests/v1/uploads_spec.rb index 7663eac253f..5c70fcf9c1b 100644 --- a/modules/simple_forms_api/spec/requests/v1/uploads_spec.rb +++ b/modules/simple_forms_api/spec/requests/v1/uploads_spec.rb @@ -31,92 +31,105 @@ ] describe '#submit' do - let(:metadata_file) { "#{file_seed}.SimpleFormsApi.metadata.json" } - let(:file_seed) { 'tmp/some-unique-simple-forms-file-seed' } + context 'going to Lighthouse Benefits Intake API' do + let(:metadata_file) { "#{file_seed}.SimpleFormsApi.metadata.json" } + let(:file_seed) { 'tmp/some-unique-simple-forms-file-seed' } - before { allow(Common::FileHelpers).to receive(:random_file_path).and_return(file_seed) } + before do + VCR.insert_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') + VCR.insert_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') + allow(Common::FileHelpers).to receive(:random_file_path).and_return(file_seed) + end - after { Common::FileHelpers.delete_file_if_exists(metadata_file) } + after do + VCR.eject_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') + VCR.eject_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') + Common::FileHelpers.delete_file_if_exists(metadata_file) + end - non_ivc_forms.each do |form| - fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', form) - data = JSON.parse(fixture_path.read) + non_ivc_forms.each do |form| + fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', form) + data = JSON.parse(fixture_path.read) - it 'makes the request' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do - allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) + it 'makes the request' do + allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) - post '/simple_forms_api/v1/simple_forms', params: data + post '/simple_forms_api/v1/simple_forms', params: data - expect(SimpleFormsApiSubmission::MetadataValidator).to have_received(:validate) - expect(response).to have_http_status(:ok) - end + expect(SimpleFormsApiSubmission::MetadataValidator).to have_received(:validate) + expect(response).to have_http_status(:ok) + end + + it 'saves a FormSubmissionAttempt' do + allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) + + expect do + post '/simple_forms_api/v1/simple_forms', params: data + end.to change(FormSubmissionAttempt, :count).by(1) end end - it 'saves a FormSubmissionAttempt' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do + authenticated_non_ivc_forms.each do |form| + fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', form) + data = JSON.parse(fixture_path.read) + + context 'authenticated user' do + before do + user = create(:user) + sign_in_as(user) + create(:in_progress_form, user_uuid: user.uuid, form_id: data['form_number']) + end + + it 'clears the InProgressForm' do allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) expect do post '/simple_forms_api/v1/simple_forms', params: data - end.to change(FormSubmissionAttempt, :count).by(1) + end.to change(InProgressForm, :count).by(-1) end end end - end - authenticated_non_ivc_forms.each do |form| - fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', form) - data = JSON.parse(fixture_path.read) + context 'request with intent to file' do + context 'authenticated' do + before do + sign_in + allow_any_instance_of(User).to receive(:icn).and_return('123498767V234859') + allow_any_instance_of(Auth::ClientCredentials::Service).to receive(:get_token).and_return('fake_token') + end - context 'authenticated user' do - before do - user = create(:user) - sign_in_as(user) - create(:in_progress_form, user_uuid: user.uuid, form_id: data['form_number']) - end + context 'third party' do + let(:expiration_date) { Time.zone.now } - it 'clears the InProgressForm' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do - allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) + before do + allow_any_instance_of(ActiveSupport::TimeZone).to receive(:now).and_return(expiration_date) + end + + %w[THIRD_PARTY_VETERAN THIRD_PARTY_SURVIVING_DEPENDENT].each do |identification| + it 'returns an expiration date' do + fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', + 'vba_21_0966.json') + data = JSON.parse(fixture_path.read) + data['preparer_identification'] = identification - expect do post '/simple_forms_api/v1/simple_forms', params: data - end.to change(InProgressForm, :count).by(-1) + + parsed_response_body = JSON.parse(response.body) + parsed_expiration_date = Time.zone.parse(parsed_response_body['expiration_date']) + expect(parsed_expiration_date.to_s).to eq (expiration_date + 1.year).to_s + end end end end - end - end - ivc_forms.each do |form| - fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', form) - data = JSON.parse(fixture_path.read) - - it 'uploads a PDF file to S3' do - allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) - allow_any_instance_of(Aws::S3::Client).to receive(:put_object).and_return(true) - - post '/simple_forms_api/v1/simple_forms', params: data - - expect(response).to have_http_status(:ok) - end - end - - describe 'request with intent to file unauthenticated' do - let(:expiration_date) { Time.zone.now } + context 'unauthenticated' do + let(:expiration_date) { Time.zone.now } - before do - allow_any_instance_of(ActiveSupport::TimeZone).to receive(:now).and_return(expiration_date) - end + before do + allow_any_instance_of(ActiveSupport::TimeZone).to receive(:now).and_return(expiration_date) + end - it 'returns an expiration date' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do + it 'returns an expiration date' do fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', 'vba_21_0966.json') data = JSON.parse(fixture_path.read) @@ -129,112 +142,133 @@ end end end - end - describe 'authenticated' do - before do - sign_in - allow_any_instance_of(User).to receive(:icn).and_return('123498767V234859') - allow_any_instance_of(Auth::ClientCredentials::Service).to receive(:get_token).and_return('fake_token') - end - - describe 'request with intent to file' do - describe 'veteran' do - it 'makes the request with an intent to file' do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/200_response_survivor') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/create_compensation_200_response') do - fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', - 'vba_21_0966-min.json') - data = JSON.parse(fixture_path.read) - data['preparer_identification'] = 'VETERAN' - - post '/simple_forms_api/v1/simple_forms', params: data - - expect(response).to have_http_status(:ok) - end - end - end - end - end + context 'request with attached documents' do + it 'appends the attachments to the 40-0247 PDF' do + fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', + 'vba_40_0247_with_supporting_document.json') + pdf_path = Rails.root.join('spec', 'fixtures', 'files', 'doctors-note.pdf') + data = JSON.parse(fixture_path.read) + attachment = double + allow(attachment).to receive(:to_pdf).and_return(pdf_path) + + expect(PersistentAttachment).to receive(:where).with(guid: ['a-random-uuid']).and_return([attachment]) + + post '/simple_forms_api/v1/simple_forms', params: data + + expect(response).to have_http_status(:ok) end - describe 'third party' do - let(:expiration_date) { Time.zone.now } + it 'appends the attachments to the 40-10007 PDF' do + fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', + 'vba_40_10007_with_supporting_document.json') + pdf_path = Rails.root.join('spec', 'fixtures', 'files', 'doctors-note.pdf') + data = JSON.parse(fixture_path.read) + attachment = double + allow(attachment).to receive(:to_pdf).and_return(pdf_path) + expect(PersistentAttachment).to receive(:where).with(guid: ['a-random-uuid']).and_return([attachment]) + post '/simple_forms_api/v1/simple_forms', params: data + expect(response).to have_http_status(:ok) + end + end - before do - allow_any_instance_of(ActiveSupport::TimeZone).to receive(:now).and_return(expiration_date) - end + context 'LOA3 authenticated' do + before do + sign_in + allow_any_instance_of(User).to receive(:icn).and_return('123498767V234859') + allow_any_instance_of(Auth::ClientCredentials::Service).to receive(:get_token).and_return('fake_token') + end - %w[THIRD_PARTY_VETERAN THIRD_PARTY_SURVIVING_DEPENDENT].each do |identification| - it 'returns an expiration date' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do - fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', - 'vba_21_0966.json') - data = JSON.parse(fixture_path.read) - data['preparer_identification'] = identification - - post '/simple_forms_api/v1/simple_forms', params: data - - parsed_response_body = JSON.parse(response.body) - parsed_expiration_date = Time.zone.parse(parsed_response_body['expiration_date']) - expect(parsed_expiration_date.to_s).to eq (expiration_date + 1.year).to_s - end - end - end - end + it 'stamps the LOA3 text on the PDF' do + fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', + 'vba_21_4142.json') + data = JSON.parse(fixture_path.read) + + allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) + expect_any_instance_of(SimpleFormsApi::PdfFiller).to receive(:generate).with(3) + + post '/simple_forms_api/v1/simple_forms', params: data end end - it 'stamps the LOA3 text on the PDF' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do + context 'transliterating fields' do + context 'transliteration succeeds' do + it 'responds with ok' do fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', - 'vba_21_4142.json') + 'form_with_accented_chars_21_0966.json') data = JSON.parse(fixture_path.read) - allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) - expect_any_instance_of(SimpleFormsApi::PdfFiller).to receive(:generate).with(3) post '/simple_forms_api/v1/simple_forms', params: data + + expect(response).to have_http_status(:ok) end end - end - end - describe 'request with attached documents' do - it 'appends the attachments to the 40-0247 PDF' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do + context 'transliteration fails' do + it 'responds with an error' do fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', - 'vba_40_0247_with_supporting_document.json') - pdf_path = Rails.root.join('spec', 'fixtures', 'files', 'doctors-note.pdf') + 'form_with_non_latin_chars_21_0966.json') data = JSON.parse(fixture_path.read) - attachment = double - allow(attachment).to receive(:to_pdf).and_return(pdf_path) - - expect(PersistentAttachment).to receive(:where).with(guid: ['a-random-uuid']).and_return([attachment]) post '/simple_forms_api/v1/simple_forms', params: data - expect(response).to have_http_status(:ok) + expect(response).to have_http_status(:error) + expect(response.body).to include('not compatible with the Windows-1252 character set') end end end + end - it 'appends the attachments to the 40-10007 PDF' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do - fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', - 'vba_40_10007_with_supporting_document.json') - pdf_path = Rails.root.join('spec', 'fixtures', 'files', 'doctors-note.pdf') - data = JSON.parse(fixture_path.read) - attachment = double - allow(attachment).to receive(:to_pdf).and_return(pdf_path) - expect(PersistentAttachment).to receive(:where).with(guid: ['a-random-uuid']).and_return([attachment]) - post '/simple_forms_api/v1/simple_forms', params: data - expect(response).to have_http_status(:ok) + context 'going to S3' do + ivc_forms.each do |form| + fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', form) + data = JSON.parse(fixture_path.read) + + it 'uploads a PDF file to S3' do + allow(SimpleFormsApiSubmission::MetadataValidator).to receive(:validate) + allow_any_instance_of(Aws::S3::Client).to receive(:put_object).and_return(true) + + post '/simple_forms_api/v1/simple_forms', params: data + + expect(response).to have_http_status(:ok) + end + end + end + + context 'going to Lighthouse Benefits Claims API' do + before do + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/404_response') + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/200_response_survivor') + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/create_compensation_200_response') + end + + after do + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/404_response') + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/200_response_survivor') + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/create_compensation_200_response') + end + + context 'authenticated' do + before do + sign_in + allow_any_instance_of(User).to receive(:icn).and_return('123498767V234859') + allow_any_instance_of(Auth::ClientCredentials::Service).to receive(:get_token).and_return('fake_token') + end + + context 'request with intent to file' do + context 'veteran' do + it 'makes the request with an intent to file' do + fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', + 'vba_21_0966-min.json') + data = JSON.parse(fixture_path.read) + data['preparer_identification'] = 'VETERAN' + + post '/simple_forms_api/v1/simple_forms', params: data + + expect(response).to have_http_status(:ok) + end end end end @@ -368,37 +402,6 @@ end end end - - describe 'transliterating fields' do - context 'transliteration succeeds' do - it 'responds with ok' do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do - VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do - fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', - 'form_with_accented_chars_21_0966.json') - data = JSON.parse(fixture_path.read) - - post '/simple_forms_api/v1/simple_forms', params: data - - expect(response).to have_http_status(:ok) - end - end - end - end - - context 'transliteration fails' do - it 'responds with an error' do - fixture_path = Rails.root.join('modules', 'simple_forms_api', 'spec', 'fixtures', 'form_json', - 'form_with_non_latin_chars_21_0966.json') - data = JSON.parse(fixture_path.read) - - post '/simple_forms_api/v1/simple_forms', params: data - - expect(response).to have_http_status(:error) - expect(response.body).to include('not compatible with the Windows-1252 character set') - end - end - end end describe '#submit_supporting_documents' do @@ -429,80 +432,91 @@ describe '#get_intents_to_file' do before do + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/404_response') + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/404_response_pension') + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/404_response_survivor') sign_in allow_any_instance_of(User).to receive(:icn).and_return('123498767V234859') allow_any_instance_of(Auth::ClientCredentials::Service).to receive(:get_token).and_return('fake_token') end + after do + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/404_response') + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/404_response_pension') + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/404_response_survivor') + end + describe 'no intents on file' do it 'returns no intents' do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response_pension') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response_survivor') do - get '/simple_forms_api/v1/simple_forms/get_intents_to_file' - - parsed_response = JSON.parse(response.body) - expect(parsed_response['compensation_intent']).to eq nil - expect(parsed_response['pension_intent']).to eq nil - expect(parsed_response['survivor_intent']).to eq nil - expect(response).to have_http_status(:ok) - end - end - end + get '/simple_forms_api/v1/simple_forms/get_intents_to_file' + + parsed_response = JSON.parse(response.body) + expect(parsed_response['compensation_intent']).to eq nil + expect(parsed_response['pension_intent']).to eq nil + expect(parsed_response['survivor_intent']).to eq nil + expect(response).to have_http_status(:ok) end end describe 'compensation intent on file' do + before do + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/200_response') + end + + after do + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/200_response') + end + it 'returns a compensation intent' do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/200_response') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response_pension') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response_survivor') do - get '/simple_forms_api/v1/simple_forms/get_intents_to_file' - - parsed_response = JSON.parse(response.body) - expect(parsed_response['compensation_intent']['type']).to eq 'compensation' - expect(parsed_response['pension_intent']).to eq nil - expect(parsed_response['survivor_intent']).to eq nil - expect(response).to have_http_status(:ok) - end - end - end + get '/simple_forms_api/v1/simple_forms/get_intents_to_file' + + parsed_response = JSON.parse(response.body) + expect(parsed_response['compensation_intent']['type']).to eq 'compensation' + expect(parsed_response['pension_intent']).to eq nil + expect(parsed_response['survivor_intent']).to eq nil + expect(response).to have_http_status(:ok) end end describe 'pension intent on file' do + before do + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') + end + + after do + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') + end + it 'returns a pension intent' do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response_survivor') do - get '/simple_forms_api/v1/simple_forms/get_intents_to_file' - - parsed_response = JSON.parse(response.body) - expect(parsed_response['compensation_intent']).to eq nil - expect(parsed_response['pension_intent']['type']).to eq 'pension' - expect(parsed_response['survivor_intent']).to eq nil - expect(response).to have_http_status(:ok) - end - end - end + get '/simple_forms_api/v1/simple_forms/get_intents_to_file' + + parsed_response = JSON.parse(response.body) + expect(parsed_response['compensation_intent']).to eq nil + expect(parsed_response['pension_intent']['type']).to eq 'pension' + expect(parsed_response['survivor_intent']).to eq nil + expect(response).to have_http_status(:ok) end end describe 'both intents on file' do + before do + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/200_response') + VCR.insert_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') + end + + after do + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/200_response') + VCR.eject_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') + end + it 'returns a pension intent' do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/200_response') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/200_response_pension') do - VCR.use_cassette('lighthouse/benefits_claims/intent_to_file/404_response_survivor') do - get '/simple_forms_api/v1/simple_forms/get_intents_to_file' - - parsed_response = JSON.parse(response.body) - expect(parsed_response['compensation_intent']['type']).to eq 'compensation' - expect(parsed_response['pension_intent']['type']).to eq 'pension' - expect(parsed_response['survivor_intent']).to eq nil - expect(response).to have_http_status(:ok) - end - end - end + get '/simple_forms_api/v1/simple_forms/get_intents_to_file' + + parsed_response = JSON.parse(response.body) + expect(parsed_response['compensation_intent']['type']).to eq 'compensation' + expect(parsed_response['pension_intent']['type']).to eq 'pension' + expect(parsed_response['survivor_intent']).to eq nil + expect(response).to have_http_status(:ok) end end end From 01f3bcdb61163bc112306423e5e7a0e0a9d227a1 Mon Sep 17 00:00:00 2001 From: Dan Hinze Date: Wed, 17 Apr 2024 12:42:09 -0500 Subject: [PATCH 2/7] BTSSS-77372 Add mocked responses for BTSSS (#16231) * Revert to original token URL in service * Add authorized ping mock, too * Handle Bearer Token parsing failures gracefully * Clean up services config * Revert some accidental deletions * Final bit of cleanup * Switch to correct file path * Move authorize method to a before_action * Update mockdata paths * Fix some linting errors --- config/betamocks/services_config.yml | 16 ++++++++++++++++ config/settings.yml | 1 + .../travel_pay/application_controller.rb | 10 ++++++++++ .../controllers/travel_pay/claims_controller.rb | 3 ++- .../controllers/travel_pay/pings_controller.rb | 2 ++ .../travel_pay/app/services/travel_pay/client.rb | 6 +++--- 6 files changed, 34 insertions(+), 4 deletions(-) diff --git a/config/betamocks/services_config.yml b/config/betamocks/services_config.yml index d0d17e6763f..0c8d6f0e546 100644 --- a/config/betamocks/services_config.yml +++ b/config/betamocks/services_config.yml @@ -9,6 +9,18 @@ :path: <%= "/#{Settings.ask_va_api.crm_api.veis_api_path}/ping" %> :file_path: "/ask_va/dynamics_api" :response_delay: 15 + - :method: :get + :path: "/veis/api/btsss/travelclaim/api/v1/Sample/ping" + :file_path: "/travel_pay/ping/default" + :response_delay: 0.3 + - :method: :get + :path: "/veis/api/btsss/travelclaim/api/v1/Sample/authorized-ping" + :file_path: "/travel_pay/ping/default" + :response_delay: 0.3 + - :method: :post + :path: "/veis/api/btsss/travelclaim/api/v1/Auth/access-token" + :file_path: "/travel_pay/token/default" + :response_delay: 0.3 - :method: :post :path: <%= "/#{Settings.ask_va_api.crm_api.veis_api_path}/inquiries/new" %> :file_path: "/ask_va/crm_api/post_inquiries/default" @@ -22,6 +34,10 @@ :path: <%= "/#{Settings.ask_va_api.crm_api.tenant_id}/oauth2/v2.0/token" %> :file_path: "/ask_va/token/default" :response_delay: 0.3 + - :method: :post + :path: <%= "/#{Settings.travel_pay.veis.tenant_id}/oauth2/token" %> + :file_path: "/travel_pay/token/default" + :response_delay: 0.3 - :name: 'carma' :base_uri: <%= "#{URI(Settings['salesforce-carma'].url).host}:#{URI(Settings['salesforce-carma'].url).port}" %> diff --git a/config/settings.yml b/config/settings.yml index 88fac504297..c2aecf2c853 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -1666,6 +1666,7 @@ brd: travel_pay: + mock: true veis: client_id: ~ client_secret: ~ diff --git a/modules/travel_pay/app/controllers/travel_pay/application_controller.rb b/modules/travel_pay/app/controllers/travel_pay/application_controller.rb index 7e522c1a4a1..ae4696eb0a0 100644 --- a/modules/travel_pay/app/controllers/travel_pay/application_controller.rb +++ b/modules/travel_pay/app/controllers/travel_pay/application_controller.rb @@ -38,6 +38,16 @@ def after_logger logger.info('travel-pay') { Utils::Logger.build(self).after } end + def authorize + auth_header = request.headers['Authorization'] + raise_unauthorized('Missing Authorization header') if auth_header.nil? + raise_unauthorized('Authorization header missing Bearer token') unless auth_header.start_with?('Bearer ') + end + + def raise_unauthorized(detail) + raise Common::Exceptions::Unauthorized.new(detail:) + end + # Blocks requests from being handled if feature flag is disabled def block_if_flag_disabled unless Flipper.enabled?(:travel_pay_power_switch, @current_user) diff --git a/modules/travel_pay/app/controllers/travel_pay/claims_controller.rb b/modules/travel_pay/app/controllers/travel_pay/claims_controller.rb index 413d9ea4a68..18136f9f7e6 100644 --- a/modules/travel_pay/app/controllers/travel_pay/claims_controller.rb +++ b/modules/travel_pay/app/controllers/travel_pay/claims_controller.rb @@ -2,9 +2,10 @@ module TravelPay class ClaimsController < ApplicationController + before_action :authorize + def index veis_token = client.request_veis_token - # Non-intuitive Ruby behavior: #split splits a string on space by default vagov_token = request.headers['Authorization'].split[1] btsss_token = client.request_btsss_token(veis_token, vagov_token) diff --git a/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb b/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb index 8529b72d842..c0d12814307 100644 --- a/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb +++ b/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb @@ -2,6 +2,8 @@ module TravelPay class PingsController < ApplicationController + before_action :authorize, only: [:authorized_ping] + def ping veis_token = client.request_veis_token diff --git a/modules/travel_pay/app/services/travel_pay/client.rb b/modules/travel_pay/app/services/travel_pay/client.rb index 7cafe4a677f..e48b5f52c3a 100644 --- a/modules/travel_pay/app/services/travel_pay/client.rb +++ b/modules/travel_pay/app/services/travel_pay/client.rb @@ -112,7 +112,7 @@ def connection(server_url:) Faraday.new(url: server_url) do |conn| conn.use :breakers conn.response :raise_error, error_prefix: service_name, include_request: true - conn.response :betamocks if use_fakes? + conn.response :betamocks if mock_enabled? conn.response :json conn.request :json @@ -123,8 +123,8 @@ def connection(server_url:) ## # Syntactic sugar for determining if the client should use # fake api responses or actually connect to the BTSSS API - def use_fakes? - Settings.useFakes + def mock_enabled? + Settings.travel_pay.mock end end end From 86b38e2bb97a03d4ce24651b3e485edeb14c52b7 Mon Sep 17 00:00:00 2001 From: Tom Harrison Date: Wed, 17 Apr 2024 15:40:35 -0400 Subject: [PATCH 3/7] Add direct deposit email notification template (#16357) * Add template_type method to VANotifyEmailJob * Update direct deposit email job to accept a default parameter of nil for the direct deposit type. --- .../v0/profile/direct_deposits_controller.rb | 2 +- app/sidekiq/va_notify_dd_email_job.rb | 13 +++- config/settings.yml | 1 + .../direct_deposits_controller_spec.rb | 2 +- spec/sidekiq/va_notify_dd_email_job_spec.rb | 71 +++++++++++++++---- 5 files changed, 71 insertions(+), 18 deletions(-) diff --git a/app/controllers/v0/profile/direct_deposits_controller.rb b/app/controllers/v0/profile/direct_deposits_controller.rb index 4d4f6b92316..942a666f594 100644 --- a/app/controllers/v0/profile/direct_deposits_controller.rb +++ b/app/controllers/v0/profile/direct_deposits_controller.rb @@ -88,7 +88,7 @@ def control_info_params end def send_confirmation_email - VANotifyDdEmailJob.send_to_emails(current_user.all_emails, 'comp_and_pen') + VANotifyDdEmailJob.send_to_emails(current_user.all_emails) end end end diff --git a/app/sidekiq/va_notify_dd_email_job.rb b/app/sidekiq/va_notify_dd_email_job.rb index 6a1dcdc4121..1f8a81c0697 100644 --- a/app/sidekiq/va_notify_dd_email_job.rb +++ b/app/sidekiq/va_notify_dd_email_job.rb @@ -10,7 +10,7 @@ class VANotifyDdEmailJob STATSD_ERROR_NAME = 'worker.direct_deposit_confirmation_email.error' STATSD_SUCCESS_NAME = 'worker.direct_deposit_confirmation_email.success' - def self.send_to_emails(user_emails, dd_type) + def self.send_to_emails(user_emails, dd_type = nil) if user_emails.present? user_emails.each do |email| perform_async(email, dd_type) @@ -25,9 +25,9 @@ def self.send_to_emails(user_emails, dd_type) end end - def perform(email, dd_type) + def perform(email, dd_type = nil) notify_client = VaNotify::Service.new(Settings.vanotify.services.va_gov.api_key) - template_type = "direct_deposit_#{dd_type.to_sym == :ch33 ? 'edu' : 'comp_pen'}" + template_type = template_type(dd_type) template_id = Settings.vanotify.services.va_gov.template_id.public_send(template_type) notify_client.send_email( @@ -39,6 +39,13 @@ def perform(email, dd_type) handle_errors(e) end + def template_type(dd_type) + return 'direct_deposit_edu' if dd_type&.to_sym == :ch33 + return 'direct_deposit_comp_pen' if dd_type&.to_sym == :comp_pen + + 'direct_deposit' + end + def handle_errors(ex) VANotifyDdEmailJob.log_exception_to_sentry(ex) StatsD.increment(STATSD_ERROR_NAME) diff --git a/config/settings.yml b/config/settings.yml index c2aecf2c853..4ca4465d021 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -1281,6 +1281,7 @@ vanotify: in_progress_reminder_email_generic: fake_template_id covid_vaccine_registration: fake_template_id covid_vaccine_expanded_registration: fake_template_id + direct_deposit: direct_deposit_template_id direct_deposit_edu: edu_template_id direct_deposit_comp_pen: comp_pen_template_id login_reactivation_email: reactivation_email_test_b diff --git a/spec/controllers/v0/profile/direct_deposits_controller_spec.rb b/spec/controllers/v0/profile/direct_deposits_controller_spec.rb index 1e0af7acae2..3dd8abc3e91 100644 --- a/spec/controllers/v0/profile/direct_deposits_controller_spec.rb +++ b/spec/controllers/v0/profile/direct_deposits_controller_spec.rb @@ -181,7 +181,7 @@ context 'when the user does have an associated email address' do it 'sends an email through va notify' do expect(VANotifyDdEmailJob).to receive(:send_to_emails).with( - user.all_emails, 'comp_and_pen' + user.all_emails ) VCR.use_cassette('lighthouse/direct_deposit/update/200_valid') do diff --git a/spec/sidekiq/va_notify_dd_email_job_spec.rb b/spec/sidekiq/va_notify_dd_email_job_spec.rb index 10a5d528e74..406ee454aa7 100644 --- a/spec/sidekiq/va_notify_dd_email_job_spec.rb +++ b/spec/sidekiq/va_notify_dd_email_job_spec.rb @@ -38,19 +38,42 @@ describe '#perform' do let(:notification_client) { double('Notifications::Client') } - %w[ch33 comp_pen].each do |dd_type| - context "with a dd type of #{dd_type}" do - it 'sends a confirmation email' do - allow(VaNotify::Service).to receive(:new) - .with(Settings.vanotify.services.va_gov.api_key).and_return(notification_client) - - expect(notification_client).to receive(:send_email).with( - email_address: email, - template_id: dd_type == 'ch33' ? 'edu_template_id' : 'comp_pen_template_id' - ) - - described_class.new.perform(email, dd_type) - end + context 'with a dd type of ch33' do + it 'sends a confirmation email using the edu template' do + allow(VaNotify::Service).to receive(:new) + .with(Settings.vanotify.services.va_gov.api_key).and_return(notification_client) + + expect(notification_client).to receive(:send_email).with( + email_address: email, template_id: 'edu_template_id' + ) + + described_class.new.perform(email, 'ch33') + end + end + + context 'with a dd type of comp_pen' do + it 'sends a confirmation email using the comp and pen template' do + allow(VaNotify::Service).to receive(:new) + .with(Settings.vanotify.services.va_gov.api_key).and_return(notification_client) + + expect(notification_client).to receive(:send_email).with( + email_address: email, template_id: 'comp_pen_template_id' + ) + + described_class.new.perform(email, 'comp_pen') + end + end + + context 'without a dd type' do + it 'sends a confirmation email using the direct_deposit template' do + allow(VaNotify::Service).to receive(:new) + .with(Settings.vanotify.services.va_gov.api_key).and_return(notification_client) + + expect(notification_client).to receive(:send_email).with( + email_address: email, template_id: 'direct_deposit_template_id' + ) + + described_class.new.perform(email, nil) end end @@ -89,4 +112,26 @@ .and trigger_statsd_increment('worker.direct_deposit_confirmation_email.error') end end + + describe '#get_template' do + let(:job) { VANotifyDdEmailJob.new } + + context 'when dd_type is nil' do + it 'returns the direct_deposit template' do + expect(job.template_type(nil)).to eq('direct_deposit') + end + end + + context 'when dd_type is comp_pen' do + it 'returns the direct_deposit template' do + expect(job.template_type('comp_pen')).to eq('direct_deposit_comp_pen') + end + end + + context 'when dd_type is edu' do + it 'returns the direct_deposit template' do + expect(job.template_type('edu')).to eq('direct_deposit') + end + end + end end From 3a3ec61e4dd099d6ec9be0f3853adc6c7cb612b7 Mon Sep 17 00:00:00 2001 From: Kevin Duensing Date: Wed, 17 Apr 2024 16:08:40 -0400 Subject: [PATCH 4/7] Add method to request a token from STS (#16372) * Add method to request a token from STS * fix rubocop * Fix tests * Remove debugging line * change parameter to reflect actual value * Merge conflict fix * remove diff file from merge conflict --- .../travel_pay/claims_controller.rb | 6 +- .../travel_pay/pings_controller.rb | 4 +- .../app/services/travel_pay/client.rb | 57 ++++++++++++++++++- .../controllers/claims_controller_spec.rb | 12 +++- .../spec/controllers/pings_controller_spec.rb | 3 + 5 files changed, 73 insertions(+), 9 deletions(-) diff --git a/modules/travel_pay/app/controllers/travel_pay/claims_controller.rb b/modules/travel_pay/app/controllers/travel_pay/claims_controller.rb index 18136f9f7e6..59cce20d36d 100644 --- a/modules/travel_pay/app/controllers/travel_pay/claims_controller.rb +++ b/modules/travel_pay/app/controllers/travel_pay/claims_controller.rb @@ -6,9 +6,9 @@ class ClaimsController < ApplicationController def index veis_token = client.request_veis_token - # Non-intuitive Ruby behavior: #split splits a string on space by default - vagov_token = request.headers['Authorization'].split[1] - btsss_token = client.request_btsss_token(veis_token, vagov_token) + + sts_token = client.request_sts_token(@current_user) + btsss_token = client.request_btsss_token(veis_token, sts_token) begin claims = client.get_claims(veis_token, btsss_token) diff --git a/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb b/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb index c0d12814307..6f8964a7fd2 100644 --- a/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb +++ b/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb @@ -13,9 +13,9 @@ def ping end def authorized_ping - vagov_token = request.headers['Authorization'].split[1] + sts_token = client.request_sts_token(@current_user) veis_token = client.request_veis_token - btsss_token = client.request_btsss_token(veis_token, vagov_token) + btsss_token = client.request_btsss_token(veis_token, sts_token) btsss_authorized_ping_response = client.authorized_ping(veis_token, btsss_token) render json: { diff --git a/modules/travel_pay/app/services/travel_pay/client.rb b/modules/travel_pay/app/services/travel_pay/client.rb index e48b5f52c3a..24ef261c2c9 100644 --- a/modules/travel_pay/app/services/travel_pay/client.rb +++ b/modules/travel_pay/app/services/travel_pay/client.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'securerandom' + module TravelPay class Client ## @@ -24,7 +26,7 @@ def request_veis_token # # @return [Faraday::Response] # - def request_btsss_token(veis_token, vagov_token) + def request_btsss_token(veis_token, sts_token) btsss_url = Settings.travel_pay.base_url api_key = Settings.travel_pay.subscription_key client_number = Settings.travel_pay.client_number @@ -33,7 +35,7 @@ def request_btsss_token(veis_token, vagov_token) req.headers['Authorization'] = "Bearer #{veis_token}" req.headers['Ocp-Apim-Subscription-Key'] = api_key req.headers['BTSSS-API-Client-Number'] = client_number.to_s - req.body = { authJwt: vagov_token } + req.body = { authJwt: sts_token } end response.body['access_token'] end @@ -90,8 +92,59 @@ def get_claims(veis_token, btsss_token) symbolized_body[:data].sort_by(&parse_claim_date).reverse! end + def request_sts_token(user) + host_baseurl = build_host_baseurl({ ip_form: false }) + private_key_file = Settings.sign_in.sts_client.key_path + private_key = OpenSSL::PKey::RSA.new(File.read(private_key_file)) + + assertion = build_sts_assertion(user) + jwt = JWT.encode(assertion, private_key, 'RS256') + + # send to sis + response = connection(server_url: host_baseurl).post('/v0/sign_in/token') do |req| + req.params['grant_type'] = 'urn:ietf:params:oauth:grant-type:jwt-bearer' + req.params['assertion'] = jwt + end + + response.body['data']['access_token'] + end + private + def build_sts_assertion(user) + service_account_id = Settings.travel_pay.sts.service_account_id + host_baseurl = build_host_baseurl({ ip_form: false }) + audience_baseurl = build_host_baseurl({ ip_form: true }) + + current_time = Time.now.to_i + jti = SecureRandom.uuid + + { + 'iss' => host_baseurl, + 'sub' => user.email, + 'aud' => "#{audience_baseurl}/v0/sign_in/token", + 'iat' => current_time, + 'exp' => current_time + 300, + 'scopes' => [], + 'service_account_id' => service_account_id, + 'jti' => jti, + 'user_attributes' => { 'icn' => user.icn } + } + end + + def build_host_baseurl(config) + env = Settings.vsp_environment + host = Settings.hostname + + if env == 'localhost' + return 'http://127.0.0.1:3000' if config[:ip_form] + + 'http://localhost:3000' + end + + "https://#{host}" + end + def veis_params { client_id: Settings.travel_pay.veis.client_id, diff --git a/modules/travel_pay/spec/controllers/claims_controller_spec.rb b/modules/travel_pay/spec/controllers/claims_controller_spec.rb index 0762820acf5..16c2ca24639 100644 --- a/modules/travel_pay/spec/controllers/claims_controller_spec.rb +++ b/modules/travel_pay/spec/controllers/claims_controller_spec.rb @@ -12,9 +12,13 @@ .to receive(:request_veis_token) .and_return('veis_token') + allow_any_instance_of(TravelPay::Client) + .to receive(:request_sts_token) + .and_return('sts_token') + allow_any_instance_of(TravelPay::Client) .to receive(:request_btsss_token) - .with('veis_token', 'vagov_token') + .with('veis_token', 'sts_token') .and_return('btsss_token') allow_any_instance_of(TravelPay::Client) @@ -34,9 +38,13 @@ .to receive(:request_veis_token) .and_return('veis_token') + allow_any_instance_of(TravelPay::Client) + .to receive(:request_sts_token) + .and_return('sts_token') + allow_any_instance_of(TravelPay::Client) .to receive(:request_btsss_token) - .with('veis_token', 'vagov_token') + .with('veis_token', 'sts_token') .and_return('btsss_token') allow_any_instance_of(TravelPay::Client) diff --git a/modules/travel_pay/spec/controllers/pings_controller_spec.rb b/modules/travel_pay/spec/controllers/pings_controller_spec.rb index 750a96600bf..93c43f00240 100644 --- a/modules/travel_pay/spec/controllers/pings_controller_spec.rb +++ b/modules/travel_pay/spec/controllers/pings_controller_spec.rb @@ -48,6 +48,9 @@ before do btsss_authorized_ping_response = double allow(btsss_authorized_ping_response).to receive(:status).and_return(200) + allow(client) + .to receive(:request_sts_token) + .and_return('sample_sts_token') allow(client) .to receive(:request_btsss_token) .and_return('sample_btsss_token') From 21d1546b4211119f6aa634e4f2270a4bc90c2b1a Mon Sep 17 00:00:00 2001 From: Trevor Bosaw Date: Wed, 17 Apr 2024 13:14:09 -0700 Subject: [PATCH 5/7] [80904] Creating a terms of use application check that occurs during secondary authentications with custom (#16371) --- lib/saml/post_url_service.rb | 29 +++++- spec/lib/saml/post_url_service_spec.rb | 129 ++++++++++++++++++------- 2 files changed, 121 insertions(+), 37 deletions(-) diff --git a/lib/saml/post_url_service.rb b/lib/saml/post_url_service.rb index d16313784d4..b91b651d8e0 100644 --- a/lib/saml/post_url_service.rb +++ b/lib/saml/post_url_service.rb @@ -63,8 +63,7 @@ def tou_declined_logout_redirect_url end def terms_of_use_redirect_url - application = @tracker&.payload_attr(:application) || 'vaweb' - if enabled_tou_clients.include?(application) + if terms_of_use_enabled_application Rails.logger.info('Redirecting to /terms-of-use', type: :ssoe) add_query(terms_of_use_url, { redirect_url: login_redirect_url }) else @@ -79,6 +78,32 @@ def ssoe_slo_url private + def terms_of_use_enabled_application + cache_key = "terms_of_use_redirect_user_#{user.uuid}" + cached_application = retrieve_and_delete_terms_of_use_redirect_user(cache_key) + current_application = @tracker&.payload_attr(:application) + write_terms_of_use_redirect_user(cache_key, current_application) if should_cache_application?(current_application) + terms_of_use_redirect_enabled?(cached_application, current_application) + end + + def terms_of_use_redirect_enabled?(cached_application, current_application) + enabled_tou_clients.include?(cached_application || current_application || 'vaweb') + end + + def should_cache_application?(application) + enabled_tou_clients.include?(application) + end + + def retrieve_and_delete_terms_of_use_redirect_user(cache_key) + application = Rails.cache.read(cache_key) + Rails.cache.delete(cache_key) + application + end + + def write_terms_of_use_redirect_user(cache_key, application) + Rails.cache.write(cache_key, application, expires_in: 5.minutes) + end + def terms_of_use_url if Settings.review_instance_slug.present? "http://#{Settings.review_instance_slug}.review.vetsgov-internal/terms-of-use" diff --git a/spec/lib/saml/post_url_service_spec.rb b/spec/lib/saml/post_url_service_spec.rb index 692020efd7a..fb2c9df8172 100644 --- a/spec/lib/saml/post_url_service_spec.rb +++ b/spec/lib/saml/post_url_service_spec.rb @@ -613,29 +613,102 @@ let(:expected_log_message) { 'Redirecting to /terms-of-use' } let(:expected_log_payload) { { type: :ssoe } } - context 'when tracker application is within Settings.terms_of_use.enabled_clients' do + context 'when associated terms of use redirect user cache object exists' do + let(:cache_key) { "terms_of_use_redirect_user_#{user.uuid}" } + let(:enabled_clients) { application } + let(:cache_expiration) { 5.minutes } + before do - allow(Settings.terms_of_use).to receive(:enabled_clients).and_return(application) + allow(Settings.terms_of_use).to receive(:enabled_clients).and_return(enabled_clients) + allow(Rails.cache).to receive(:read).with(cache_key).and_return(application) end - context 'and authentication is occuring on a review instance' do - let(:review_instance_slug) { 'some-review-instance-slug' } - let(:review_instance_url) { "#{review_instance_slug}.review.vetsgov-internal" } + context 'and application is within Settings.terms_of_use.enabled_clients' do + let(:enabled_clients) { application } - before { allow(Settings).to receive(:review_instance_slug).and_return(review_instance_slug) } + context 'and authentication is occuring on a review instance' do + let(:review_instance_slug) { 'some-review-instance-slug' } + let(:review_instance_url) { "#{review_instance_slug}.review.vetsgov-internal" } - it 'has a login redirect url as a parameter embedded in review instance terms of use page' do - expect(subject.terms_of_use_redirect_url) - .to eq("http://#{review_instance_url}/terms-of-use?#{expected_redirect_url_param}") + before { allow(Settings).to receive(:review_instance_slug).and_return(review_instance_slug) } + + it 'has a login redirect url as a parameter embedded in review instance terms of use page' do + expect(subject.terms_of_use_redirect_url) + .to eq("http://#{review_instance_url}/terms-of-use?#{expected_redirect_url_param}") + end + + it 'logs expected message and payload' do + expect(Rails.logger).to receive(:info).with(expected_log_message, expected_log_payload) + subject.terms_of_use_redirect_url + end end - it 'logs expected message and payload' do - expect(Rails.logger).to receive(:info).with(expected_log_message, expected_log_payload) - subject.terms_of_use_redirect_url + context 'and authentication is not occurring on a review instance' do + it 'has a login redirect url as a parameter embedded in terms of use page with success' do + expect(subject.terms_of_use_redirect_url) + .to eq("#{values[:base_redirect]}/terms-of-use?#{expected_redirect_url_param}") + end + + it 'logs expected message and payload' do + expect(Rails.logger).to receive(:info).with(expected_log_message, expected_log_payload) + subject.terms_of_use_redirect_url + end end end - context 'and authentication is not occurring on a review instance' do + context 'and stored application is not within Settings.terms_of_use.enabled_clients' do + let(:enabled_clients) { '' } + + it 'has a login redirect url with success not embedded in a terms of use page' do + expect(subject.terms_of_use_redirect_url).to eq(expected_login_redirect_url) + end + end + + it 'deletes the cached terms of use redirect user object' do + expect(Rails.cache).to receive(:delete).with(cache_key) + subject.terms_of_use_redirect_url + end + end + + context 'when associated terms of use redirect user cache object does not exist' do + context 'when tracker application is within Settings.terms_of_use.enabled_clients' do + before do + allow(Settings.terms_of_use).to receive(:enabled_clients).and_return(application) + end + + context 'and authentication is occuring on a review instance' do + let(:review_instance_slug) { 'some-review-instance-slug' } + let(:review_instance_url) { "#{review_instance_slug}.review.vetsgov-internal" } + + before { allow(Settings).to receive(:review_instance_slug).and_return(review_instance_slug) } + + it 'has a login redirect url as a parameter embedded in review instance terms of use page' do + expect(subject.terms_of_use_redirect_url) + .to eq("http://#{review_instance_url}/terms-of-use?#{expected_redirect_url_param}") + end + + it 'logs expected message and payload' do + expect(Rails.logger).to receive(:info).with(expected_log_message, expected_log_payload) + subject.terms_of_use_redirect_url + end + end + + context 'and authentication is not occurring on a review instance' do + it 'has a login redirect url as a parameter embedded in terms of use page with success' do + expect(subject.terms_of_use_redirect_url) + .to eq("#{values[:base_redirect]}/terms-of-use?#{expected_redirect_url_param}") + end + + it 'logs expected message and payload' do + expect(Rails.logger).to receive(:info).with(expected_log_message, expected_log_payload) + subject.terms_of_use_redirect_url + end + end + end + + context 'when tracker application is nil' do + let(:application) { nil } + it 'has a login redirect url as a parameter embedded in terms of use page with success' do expect(subject.terms_of_use_redirect_url) .to eq("#{values[:base_redirect]}/terms-of-use?#{expected_redirect_url_param}") @@ -646,30 +719,16 @@ subject.terms_of_use_redirect_url end end - end - - context 'when tracker application is nil' do - let(:application) { nil } - - it 'has a login redirect url as a parameter embedded in terms of use page with success' do - expect(subject.terms_of_use_redirect_url) - .to eq("#{values[:base_redirect]}/terms-of-use?#{expected_redirect_url_param}") - end - - it 'logs expected message and payload' do - expect(Rails.logger).to receive(:info).with(expected_log_message, expected_log_payload) - subject.terms_of_use_redirect_url - end - end - context 'when tracker application is not within Settings.terms_of_use.enabled_clients' do - before do - allow(Settings.terms_of_use).to receive(:enabled_clients).and_return('') - end + context 'when tracker application is not within Settings.terms_of_use.enabled_clients' do + before do + allow(Settings.terms_of_use).to receive(:enabled_clients).and_return('') + end - it 'has a login redirect url with success not embedded in a terms of use page' do - expect(subject.terms_of_use_redirect_url) - .to eq(expected_login_redirect_url) + it 'has a login redirect url with success not embedded in a terms of use page' do + expect(subject.terms_of_use_redirect_url) + .to eq(expected_login_redirect_url) + end end end end From 3f1f88aab0801c4e5654e18ab6cf24ae2385e7a5 Mon Sep 17 00:00:00 2001 From: Gregg P <117232882+GcioGregg@users.noreply.github.com> Date: Wed, 17 Apr 2024 13:28:55 -0700 Subject: [PATCH 6/7] remove feature flags (#16390) --- app/sidekiq/education_form/templates/10203.erb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/sidekiq/education_form/templates/10203.erb b/app/sidekiq/education_form/templates/10203.erb index bad27173e61..cb756cfd5b1 100644 --- a/app/sidekiq/education_form/templates/10203.erb +++ b/app/sidekiq/education_form/templates/10203.erb @@ -2,11 +2,7 @@ CH33 *START* <%= form_identifier %> -<% if Settings.vsp_environment.eql?('production') -%> -JUN 2020 -<% else -%> OMB Control #: 2900-0878 -<% end -%> APPLICATION FOR EDITH NOURSE ROGERS STEM SCHOLARSHIP @@ -73,10 +69,6 @@ Applicant has POA: <%= yesno(@stem_automated_decision.poa) %> Applicant School Email Address: <%= @applicant.schoolEmailAddress %> Applicant School ID: <%= @applicant.schoolStudentId %> -<% if Settings.vsp_environment.eql?('production') -%> - Certification and Signature of Applicant -Signature of Applicant Date -<% else -%> <% if @applicant.isActiveDuty -%> As an active-duty service member, you have consulted with an Education Service Officer (ESO) regarding your education program. @@ -86,5 +78,4 @@ Signature of Applicant Date <% end -%> -<% end -%> <%= parse_with_template_path('footer') %> From be21fdead181807370d35bdd019c2a6ddab40f4f Mon Sep 17 00:00:00 2001 From: Casey Williams Date: Wed, 17 Apr 2024 13:33:53 -0700 Subject: [PATCH 7/7] API-34961 Trace appeals PII removal with Datadog (#16364) --- .../app/services/appeals_api/remove_pii.rb | 29 +++++++-------- .../services/appeals_api/remove_pii_spec.rb | 35 ++++++++++++++----- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/modules/appeals_api/app/services/appeals_api/remove_pii.rb b/modules/appeals_api/app/services/appeals_api/remove_pii.rb index 39fd91d8673..c9626c78e1c 100644 --- a/modules/appeals_api/app/services/appeals_api/remove_pii.rb +++ b/modules/appeals_api/app/services/appeals_api/remove_pii.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'ddtrace' + module AppealsApi class RemovePii include SentryLogging @@ -15,13 +17,20 @@ def initialize(form_type:) end def run! - validate_form_type! + Datadog::Tracing.trace("#{self.class.name} - #{form_type}") do + validate_form_type! - result = remove_pii! + result = remove_pii! - log_failure_to_sentry if records_were_not_cleared(result) + if result.blank? && records_to_be_expunged.present? + ids = records_to_be_expunged.pluck(:id) + msg = "Failed to remove expired #{form_type} PII from records" + Rails.logger.error(msg, ids) + AppealsApi::Slack::Messager.new({ msg:, ids: }).notify! + end - result + result + end end private @@ -49,17 +58,5 @@ def records_to_be_expunged .pii_expunge_policy end end - - def records_were_not_cleared(result) - result.blank? && records_to_be_expunged.present? - end - - def log_failure_to_sentry - log_message_to_sentry( - "Failed to expunge PII from #{form_type} (modules/appeals_api)", - :error, - ids: records_to_be_expunged.pluck(:id) - ) - end end end diff --git a/modules/appeals_api/spec/services/appeals_api/remove_pii_spec.rb b/modules/appeals_api/spec/services/appeals_api/remove_pii_spec.rb index 8599a4838f5..62d9785c9e6 100644 --- a/modules/appeals_api/spec/services/appeals_api/remove_pii_spec.rb +++ b/modules/appeals_api/spec/services/appeals_api/remove_pii_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'rails_helper' -require AppealsApi::Engine.root.join('spec', 'spec_helper.rb') def update_appeal_status(appeal, status, code: nil, detail: nil) # At the time of writing, the `update_status` method for each appeal model involves kicking off a sidekiq job to @@ -159,6 +158,32 @@ def create_misc_appeals = create_appeals + misc_appeal_types.map { |f| FactoryBo expect { AppealsApi::RemovePii.new(form_type: 'Invalid').run! }.to raise_error(ArgumentError) end + context 'when the removal fails' do + let!(:appeals) do + Timecop.freeze(100.days.ago) do + status = 'complete' + [create(:supplemental_claim, status:), create(:supplemental_claim_v0, status:)] + end + end + + before do + instance = AppealsApi::RemovePii.new(form_type: AppealsApi::SupplementalClaim) + msg = 'Failed to remove expired AppealsApi::SupplementalClaim PII from records' + expect(Rails.logger).to receive(:error).with(msg, appeals.map(&:id)) + expect_any_instance_of(AppealsApi::Slack::Messager).to receive(:notify!) + allow(instance).to receive(:remove_pii!).and_return [] + instance.run! + end + + it 'logs an error and the IDs of records whose PII failed to be removed' do + appeals.each do |appeal| + appeal.reload + expect(appeal.auth_headers).to be_present + expect(appeal.form_data).to be_present + end + end + end + it 'removes PII from HLR records needing PII removal' do day_old_has_pii_v2 = create :higher_level_review_v2, status: 'complete' day_old_has_pii_v2.update updated_at: 1.day.ago @@ -271,13 +296,5 @@ def create_misc_appeals = create_appeals + misc_appeal_types.map { |f| FactoryBo expect(week_old_has_pii_error.reload.form_data_ciphertext).to be_present end end - - it 'sends a message to sentry if the removal failed.' do - allow_any_instance_of(AppealsApi::RemovePii).to receive(:records_were_not_cleared).and_return(true) - service = AppealsApi::RemovePii.new(form_type: AppealsApi::NoticeOfDisagreement) - expect(service).to receive(:log_failure_to_sentry) - - service.run! - end end end