diff --git a/app/forms/idv/api_document_verification_form.rb b/app/forms/idv/api_document_verification_form.rb index c9cf72292e6..6094718cfd6 100644 --- a/app/forms/idv/api_document_verification_form.rb +++ b/app/forms/idv/api_document_verification_form.rb @@ -21,13 +21,20 @@ def initialize(params, liveness_checking_enabled:, analytics:) def submit throttled_else_increment - FormResponse.new( + response = FormResponse.new( success: valid?, errors: errors, extra: { remaining_attempts: remaining_attempts, }, ) + + @analytics.track_event( + Analytics::IDV_DOC_AUTH_SUBMITTED_IMAGE_UPLOAD_FORM, + response.to_h, + ) + + response end def remaining_attempts diff --git a/app/jobs/document_proofing_job.rb b/app/jobs/document_proofing_job.rb index 63516d0c661..0a678123dd8 100644 --- a/app/jobs/document_proofing_job.rb +++ b/app/jobs/document_proofing_job.rb @@ -4,7 +4,10 @@ class DocumentProofingJob < ApplicationJob queue_as :default def perform(result_id:, encrypted_arguments:, trace_id:, - liveness_checking_enabled:) + liveness_checking_enabled:, analytics_data:) + dcs = DocumentCaptureSession.find_by(result_id: result_id) + user = dcs.user + timer = JobHelpers::Timer.new decrypted_args = JSON.parse( Encryption::Encryptors::SessionEncryptor.new.decrypt(encrypted_arguments), @@ -40,12 +43,26 @@ def perform(result_id:, encrypted_arguments:, trace_id:, end end - dcs = DocumentCaptureSession.new(result_id: result_id) - dcs.store_doc_auth_result( result: proofer_result.to_h, # pii_from_doc is excluded from to_h to stop accidental logging pii: proofer_result.pii_from_doc, ) + + analytics = Analytics.new(user: user, request: nil, sp: dcs.issuer) + + remaining_attempts = Throttler::RemainingCount.call( + user.id, + :idv_acuant, + ) + + analytics.track_event( + Analytics::IDV_DOC_AUTH_SUBMITTED_IMAGE_UPLOAD_VENDOR, + proofer_result.to_h.merge( + state: proofer_result.pii_from_doc[:state], + async: true, + remaining_attempts: remaining_attempts, + ).merge(analytics_data), + ) ensure logger.info( { diff --git a/app/services/analytics.rb b/app/services/analytics.rb index a2dd5df2600..3b6815fd47e 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -12,7 +12,8 @@ def track_event(event, attributes = {}) user_id: attributes[:user_id] || user.uuid, locale: I18n.locale, } - analytics_hash.merge!(request_attributes) + + analytics_hash.merge!(request_attributes) if request ahoy.track(event, analytics_hash) register_doc_auth_step_from_analytics_event(event, attributes) @@ -21,7 +22,7 @@ def track_event(event, attributes = {}) # https://www.rubydoc.info/github/newrelic/rpm/NewRelic/Agent#add_custom_attributes-instance_method ::NewRelic::Agent.add_custom_attributes( user_id: analytics_hash[:user_id], - user_ip: request.remote_ip, + user_ip: request&.remote_ip, service_provider: sp, event_name: event, ) diff --git a/app/services/idv/actions/verify_document_action.rb b/app/services/idv/actions/verify_document_action.rb index 632c3d5faec..e2731b01e5b 100644 --- a/app/services/idv/actions/verify_document_action.rb +++ b/app/services/idv/actions/verify_document_action.rb @@ -49,6 +49,10 @@ def enqueue_job verify_document_capture_session, liveness_checking_enabled: liveness_checking_enabled?, trace_id: amzn_trace_id, + analytics_data: { + client_image_metrics: image_metadata, + browser_attributes: @flow.analytics.browser_attributes, + }, ) nil @@ -62,6 +66,20 @@ def image_params 'front_image_url', 'back_image_url', 'selfie_image_url'], ) end + + + def image_metadata + params.permit(:front_image_metadata, :back_image_metadata). + to_h. + transform_values do |str| + JSON.parse(str) + rescue JSON::ParserError + nil + end. + compact. + transform_keys { |key| key.gsub(/_image_metadata$/, '') }. + deep_symbolize_keys + end end end end diff --git a/app/services/idv/actions/verify_document_status_action.rb b/app/services/idv/actions/verify_document_status_action.rb index 55776962042..edbccd27b65 100644 --- a/app/services/idv/actions/verify_document_status_action.rb +++ b/app/services/idv/actions/verify_document_status_action.rb @@ -43,6 +43,11 @@ def process_async_state(current_async_state) def async_state_done(async_result) doc_pii_form_result = Idv::DocPiiForm.new(async_result.pii).submit + @flow.analytics.track_event( + Analytics::IDV_DOC_AUTH_SUBMITTED_PII_VALIDATION, + doc_pii_form_result.to_h.merge(remaining_attempts: remaining_attempts), + ) + delete_async if doc_pii_form_result.success? extract_pii_from_doc(async_result) @@ -56,15 +61,6 @@ def async_state_done(async_result) def process_result(async_state) add_cost(:acuant_result) if async_state.result.to_h[:billed] - @flow.analytics.track_event( - Analytics::IDV_DOC_AUTH_SUBMITTED_IMAGE_UPLOAD_VENDOR, - async_state.result.to_h.merge( - state: async_state.pii[:state], - async: true, - remaining_attempts: remaining_attempts, - client_image_metrics: nil, # Need to resolve how to capture image metrics, see LG-4488 - ), - ) end def verify_document_capture_session diff --git a/app/services/idv/agent.rb b/app/services/idv/agent.rb index 9b731f024c3..fb2101f2df5 100644 --- a/app/services/idv/agent.rb +++ b/app/services/idv/agent.rb @@ -35,7 +35,8 @@ def proof_address(document_capture_session, user_id:, issuer:, trace_id:) ) end - def proof_document(document_capture_session, liveness_checking_enabled:, trace_id:) + def proof_document(document_capture_session, liveness_checking_enabled:, trace_id:, + analytics_data:) encrypted_arguments = Encryption::Encryptors::SessionEncryptor.new.encrypt( { document_arguments: @applicant }.to_json, ) @@ -45,6 +46,7 @@ def proof_document(document_capture_session, liveness_checking_enabled:, trace_i liveness_checking_enabled: liveness_checking_enabled, result_id: document_capture_session.result_id, trace_id: trace_id, + analytics_data: analytics_data, ) end end diff --git a/spec/forms/openid_connect_token_form_spec.rb b/spec/forms/openid_connect_token_form_spec.rb index f238b840dd4..9b28fb7e83a 100644 --- a/spec/forms/openid_connect_token_form_spec.rb +++ b/spec/forms/openid_connect_token_form_spec.rb @@ -403,7 +403,7 @@ expect(submission.to_h).to include( success: false, errors: form.errors.messages, - error_details: hash_including(*form.errors.keys), + error_details: hash_including(*form.errors.attribute_names), client_id: nil, user_id: nil, ) diff --git a/spec/jobs/document_proofing_job_spec.rb b/spec/jobs/document_proofing_job_spec.rb index 07fb716812b..1461e9273c3 100644 --- a/spec/jobs/document_proofing_job_spec.rb +++ b/spec/jobs/document_proofing_job_spec.rb @@ -45,7 +45,10 @@ ) end - let(:document_capture_session) { DocumentCaptureSession.new(result_id: SecureRandom.hex) } + let(:user) { create(:user) } + let(:document_capture_session) do + DocumentCaptureSession.create(user_id: user.id, result_id: SecureRandom.hex) + end describe '.perform_later' do it 'stores results' do @@ -54,6 +57,7 @@ liveness_checking_enabled: liveness_checking_enabled, encrypted_arguments: encrypted_arguments, trace_id: trace_id, + analytics_data: {}, ) result = document_capture_session.load_doc_auth_async_result @@ -69,6 +73,7 @@ liveness_checking_enabled: liveness_checking_enabled, encrypted_arguments: encrypted_arguments, trace_id: trace_id, + analytics_data: {}, ) end diff --git a/spec/services/idv/actions/verify_document_status_action_spec.rb b/spec/services/idv/actions/verify_document_status_action_spec.rb index 6eef555e2db..69cf68dcdd7 100644 --- a/spec/services/idv/actions/verify_document_status_action_spec.rb +++ b/spec/services/idv/actions/verify_document_status_action_spec.rb @@ -3,7 +3,7 @@ describe Idv::Actions::VerifyDocumentStatusAction do include IdvHelper - let(:user) { build(:user) } + let(:user) { create(:user) } let(:session) { { 'idv/doc_auth' => {} } } let(:controller) do instance_double(Idv::DocAuthController, url_options: {}, session: session, analytics: analytics) @@ -11,9 +11,16 @@ let(:flow) { Idv::Flows::DocAuthFlow.new(controller, session, 'idv/doc_auth') } let(:analytics) { FakeAnalytics.new } let(:result) { { result: 'Passed', success: true, errors: {}, exception: nil, billed: true } } - let(:pii) { { state: 'MD' } } + let(:pii) do + { + first_name: Faker::Name.first_name, + last_name: Faker::Name.last_name, + dob: Faker::Date.birthday(min_age: IdentityConfig.store.idv_min_age_years + 1).to_s, + state: Faker::Address.state_abbr, + } + end let(:done) { true } - let(:async_state) { OpenStruct.new(result: result, pii: pii, 'done?' => done) } + let(:async_state) { OpenStruct.new(result: result, pii: pii, pii_from_doc: pii, 'done?' => done) } subject { described_class.new(flow) } @@ -22,22 +29,16 @@ before do allow(subject).to receive(:async_state).and_return(async_state) allow(subject).to receive(:add_cost).and_return(true) + allow(subject).to receive(:current_user).and_return(user) end it 'calls analytics to log the successful event' do subject.call expect(analytics).to have_logged_event( - Analytics::IDV_DOC_AUTH_SUBMITTED_IMAGE_UPLOAD_VENDOR, - result: 'Passed', + Analytics::IDV_DOC_AUTH_SUBMITTED_PII_VALIDATION, success: true, errors: {}, - exception: nil, - billed: true, - state: 'MD', - async: true, - remaining_attempts: nil, - client_image_metrics: nil, ) end end diff --git a/spec/services/phone_confirmation/confirmaton_session_spec.rb b/spec/services/phone_confirmation/confirmaton_session_spec.rb index bbca1f57e92..cf6fe547bd6 100644 --- a/spec/services/phone_confirmation/confirmaton_session_spec.rb +++ b/spec/services/phone_confirmation/confirmaton_session_spec.rb @@ -48,9 +48,10 @@ end describe '#matches_code?' do + let(:code) { OtpCodeGenerator.generate_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH) } subject do described_class.new( - code: '123456', + code: code, phone: '+1 (202) 123-4567', sent_at: Time.zone.now, delivery_method: :sms, @@ -58,11 +59,22 @@ end it 'returns true if the code matches' do - expect(subject.matches_code?('123456')).to eq(true) + expect(subject.matches_code?(code)).to eq(true) + end + + it 'returns true if the code matches with different case' do + random_case_code = code.chars.map { |c| (rand 2) == 0 ? c.downcase : c.upcase }.join + lowercase_code = code.downcase + uppercase_code = code.upcase + + expect(subject.matches_code?(random_case_code )).to eq(true) + expect(subject.matches_code?(lowercase_code)).to eq(true) + expect(subject.matches_code?(uppercase_code)).to eq(true) end it 'returns false if the code does not match' do - expect(subject.matches_code?('111111')).to eq(false) + bad_code = '1' * (TwoFactorAuthenticatable::DIRECT_OTP_LENGTH - 1) + expect(subject.matches_code?('11111')).to eq(false) end it 'uses a secure comparison' do diff --git a/spec/support/fake_analytics.rb b/spec/support/fake_analytics.rb index 370448b664d..53a7901dda6 100644 --- a/spec/support/fake_analytics.rb +++ b/spec/support/fake_analytics.rb @@ -14,6 +14,10 @@ def track_event(event, attributes = {}) def track_mfa_submit_event(_attributes) # no-op end + + def browser_attributes + {} + end end RSpec::Matchers.define :have_logged_event do |event_name, attributes|