From 526ecaa1980455d967dd4e02ea55ecc02107dd54 Mon Sep 17 00:00:00 2001 From: Matt Wagner Date: Wed, 30 Oct 2024 15:28:16 -0400 Subject: [PATCH 1/7] LG-14830 | Send ID type to AAMVA changelog: Internal, Identity Proofing, Send state ID type to AAMVA --- .../aamva/request/verification_request.rb | 25 ++++++++++ config/application.yml.default | 1 + lib/identity_config.rb | 1 + .../services/proofing/aamva/applicant_spec.rb | 2 +- .../request/verification_request_spec.rb | 46 +++++++++++++++++++ 5 files changed, 74 insertions(+), 1 deletion(-) diff --git a/app/services/proofing/aamva/request/verification_request.rb b/app/services/proofing/aamva/request/verification_request.rb index ecff76af4a4..1455904bb39 100644 --- a/app/services/proofing/aamva/request/verification_request.rb +++ b/app/services/proofing/aamva/request/verification_request.rb @@ -84,9 +84,34 @@ def add_user_provided_data_to_body inside: '//dldv:verifyDriverLicenseDataRequest', ) + add_state_id_type( + applicant.state_id_data.state_id_type, + document + ) if IdentityConfig.store.aamva_send_id_type + @body = document.to_s end + def add_state_id_type(id_type, document) + category_code = case id_type + when 'drivers_license' + 1 + when 'drivers_permit' + 2 + when 'state_id_card' + 3 + end + + if category_code + add_optional_element( + 'aa:DocumentCategoryCode', + value: category_code, + document:, + inside: '//dldv:verifyDriverLicenseDataRequest', + ) + end + end + def add_optional_element(name, value:, document:, inside: nil, after: nil) return if value.blank? diff --git a/config/application.yml.default b/config/application.yml.default index 899e69e50a7..5abbd6fe67e 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -19,6 +19,7 @@ aamva_auth_url: 'https://example.org:12345/auth/url' aamva_cert_enabled: true aamva_private_key: '' aamva_public_key: '' +aamva_send_id_type: false aamva_supported_jurisdictions: '["AL","AR","AZ","CO","CT","DC","DE","FL","GA","HI","IA","ID","IL","IN","KS","KY","MA","MD","ME","MI","MO","MS","MT","NC","ND","NE","NJ","NM","NV","OH","OR","PA","RI","SC","SD","TN","TX","VA","VT","WA","WI","WV","WY"]' aamva_verification_request_timeout: 5.0 aamva_verification_url: https://example.org:12345/verification/url diff --git a/lib/identity_config.rb b/lib/identity_config.rb index b1f5f0b572d..7a488470b9b 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -33,6 +33,7 @@ def self.store config.add(:aamva_cert_enabled, type: :boolean) config.add(:aamva_private_key, type: :string) config.add(:aamva_public_key, type: :string) + config.add(:aamva_send_id_type, type: :boolean) config.add(:aamva_supported_jurisdictions, type: :json) config.add(:aamva_verification_request_timeout, type: :float) config.add(:aamva_verification_url) diff --git a/spec/services/proofing/aamva/applicant_spec.rb b/spec/services/proofing/aamva/applicant_spec.rb index bb22b20d376..b2172ce9bd5 100644 --- a/spec/services/proofing/aamva/applicant_spec.rb +++ b/spec/services/proofing/aamva/applicant_spec.rb @@ -14,7 +14,7 @@ end describe '.from_proofer_applicant(applicant)' do - it 'should create an AAMVA applicant with necessary proofer applcant data' do + it 'should create an AAMVA applicant with necessary proofer applicant data' do aamva_applicant = described_class.from_proofer_applicant(proofer_applicant) expect(aamva_applicant.uuid).to eq(proofer_applicant[:uuid]) diff --git a/spec/services/proofing/aamva/request/verification_request_spec.rb b/spec/services/proofing/aamva/request/verification_request_spec.rb index 190d9edf374..13d73ba18bd 100644 --- a/spec/services/proofing/aamva/request/verification_request_spec.rb +++ b/spec/services/proofing/aamva/request/verification_request_spec.rb @@ -88,6 +88,52 @@ '2030-01-02', ) end + + context '#state_id_type' do + context 'when the feature flag is off' do + before do + expect(IdentityConfig.store).to receive(:aamva_send_id_type).and_return(false) + end + + it 'does not add a DocumentCategoryCode' do + applicant.state_id_data.state_id_type = 'drivers_license' + expect(subject.body).to_not include('') + end + end + + context 'when the feature flag is on' do + before do + expect(IdentityConfig.store).to receive(:aamva_send_id_type).and_return(true) + end + + context 'when the type is a Drivers License' do + it 'includes DocumentCategoryCode=1' do + applicant.state_id_data.state_id_type = 'drivers_license' + expect(subject.body).to include( + '1', + ) + end + end + + context 'when the type is a learners permit' do + it 'includes DocumentCategoryCode=2' do + applicant.state_id_data.state_id_type = 'drivers_permit' + expect(subject.body).to include( + '2', + ) + end + end + + context 'when the type is an ID Card' do + it 'includes DocumentCategoryCode=3' do + applicant.state_id_data.state_id_type = 'state_id_card' + expect(subject.body).to include( + '3', + ) + end + end + end + end end describe '#headers' do From bf22447810579ec662fd5130b46bac7e7e926180 Mon Sep 17 00:00:00 2001 From: Matt Wagner Date: Wed, 30 Oct 2024 15:47:53 -0400 Subject: [PATCH 2/7] Tests for no/invalid ID type --- .../aamva/request/verification_request_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/services/proofing/aamva/request/verification_request_spec.rb b/spec/services/proofing/aamva/request/verification_request_spec.rb index 13d73ba18bd..a4e60c1e987 100644 --- a/spec/services/proofing/aamva/request/verification_request_spec.rb +++ b/spec/services/proofing/aamva/request/verification_request_spec.rb @@ -132,6 +132,18 @@ ) end end + + context 'when the type is something invalid' do + it 'does not add a DocumentCategoryCode for nil ID type' do + applicant.state_id_data.state_id_type = nil + expect(subject.body).to_not include('') + end + + it 'does not add a DocumentCategoryCode for invalid ID types' do + applicant.state_id_data.state_id_type = 'License to Keep an Alpaca' + expect(subject.body).to_not include('') + end + end end end end From 3bc17f23e624a37d926ddfe7929acbdefe494ac7 Mon Sep 17 00:00:00 2001 From: Matt Wagner Date: Wed, 30 Oct 2024 17:34:13 -0400 Subject: [PATCH 3/7] Stop hardcoding state_id_type to 'drivers_license' This likely breaks tests. Let's see which ones. --- app/controllers/concerns/idv/verify_info_concern.rb | 2 -- app/controllers/idv/in_person/verify_info_controller.rb | 8 -------- app/controllers/idv/verify_info_controller.rb | 3 --- 3 files changed, 13 deletions(-) diff --git a/app/controllers/concerns/idv/verify_info_concern.rb b/app/controllers/concerns/idv/verify_info_concern.rb index efceb72bc06..e2608ab854e 100644 --- a/app/controllers/concerns/idv/verify_info_concern.rb +++ b/app/controllers/concerns/idv/verify_info_concern.rb @@ -19,8 +19,6 @@ def shared_update Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]). call('verify', :update, true) - set_state_id_type - ssn_rate_limiter.increment! document_capture_session = DocumentCaptureSession.create( diff --git a/app/controllers/idv/in_person/verify_info_controller.rb b/app/controllers/idv/in_person/verify_info_controller.rb index 5284d5e6881..1eb612bb664 100644 --- a/app/controllers/idv/in_person/verify_info_controller.rb +++ b/app/controllers/idv/in_person/verify_info_controller.rb @@ -57,14 +57,6 @@ def flow_param 'in_person' end - # state_id_type is hard-coded here because it's required for proofing against - # AAMVA. We're sticking with driver's license because most states don't discern - # between various ID types and driver's license is the most common one that will - # be supported. See also LG-3852 and related findings document. - def set_state_id_type - pii_from_user[:state_id_type] = 'drivers_license' unless invalid_state? - end - def invalid_state? pii_from_user.blank? end diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 9496890cf04..bc11819486b 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -64,9 +64,6 @@ def self.step_info def flow_param; end - # state ID type isn't manually set for Idv::VerifyInfoController - def set_state_id_type; end - def prev_url idv_ssn_url end From 66ca749c9c555560ce82c2022d0e24ba6a272d75 Mon Sep 17 00:00:00 2001 From: Matt Wagner Date: Fri, 1 Nov 2024 16:58:02 -0400 Subject: [PATCH 4/7] Clean up test failures --- app/services/proofing/mock/state_id_mock_client.rb | 4 ++-- .../idv/in_person/verify_info_controller_spec.rb | 9 +-------- spec/features/idv/analytics_spec.rb | 2 +- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/app/services/proofing/mock/state_id_mock_client.rb b/app/services/proofing/mock/state_id_mock_client.rb index 7f01945df57..3b86f0bff68 100644 --- a/app/services/proofing/mock/state_id_mock_client.rb +++ b/app/services/proofing/mock/state_id_mock_client.rb @@ -72,8 +72,8 @@ def invalid_state_id_number?(state_id_number) end def invalid_state_id_type?(state_id_type) - !SUPPORTED_STATE_ID_TYPES.include?(state_id_type) || - state_id_type.nil? + !SUPPORTED_STATE_ID_TYPES.include?(state_id_type) && + !state_id_type.nil? end end end diff --git a/spec/controllers/idv/in_person/verify_info_controller_spec.rb b/spec/controllers/idv/in_person/verify_info_controller_spec.rb index 4a6dbda956b..a05953e8b96 100644 --- a/spec/controllers/idv/in_person/verify_info_controller_spec.rb +++ b/spec/controllers/idv/in_person/verify_info_controller_spec.rb @@ -234,22 +234,15 @@ allow(user).to receive(:establishing_in_person_enrollment).and_return(enrollment) end - it 'sets ssn and state_id_type on pii_from_user' do + it 'sets ssn on pii_from_user' do expect(Idv::Agent).to receive(:new).with( hash_including( - state_id_type: 'drivers_license', ssn: Idp::Constants::MOCK_IDV_APPLICANT_SAME_ADDRESS_AS_ID[:ssn], consent_given_at: subject.idv_session.idv_consent_given_at, ), ).and_call_original - # our test data already has the expected value by default - subject.user_session['idv/in_person'][:pii_from_user].delete(:state_id_type) - put :update - - expect(subject.user_session['idv/in_person'][:pii_from_user][:state_id_type]). - to eq 'drivers_license' end context 'a user does not have an establishing in person enrollment associated with them' do diff --git a/spec/features/idv/analytics_spec.rb b/spec/features/idv/analytics_spec.rb index 7505f8b7717..606eb9373ec 100644 --- a/spec/features/idv/analytics_spec.rb +++ b/spec/features/idv/analytics_spec.rb @@ -153,7 +153,7 @@ vendor_name: 'ResolutionMock', vendor_workflow: nil, verified_attributes: nil }, - state_id: state_id_resolution_with_id_type, + state_id: state_id_resolution, threatmetrix: threatmetrix_response, }, }, From 6e88ae5db24fae25a1bffd1e27702cf0d4092ea9 Mon Sep 17 00:00:00 2001 From: Matt Wagner Date: Fri, 1 Nov 2024 17:01:31 -0400 Subject: [PATCH 5/7] Now with lintfixes --- .../proofing/aamva/request/verification_request.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/services/proofing/aamva/request/verification_request.rb b/app/services/proofing/aamva/request/verification_request.rb index 1455904bb39..84e25e75b34 100644 --- a/app/services/proofing/aamva/request/verification_request.rb +++ b/app/services/proofing/aamva/request/verification_request.rb @@ -84,10 +84,12 @@ def add_user_provided_data_to_body inside: '//dldv:verifyDriverLicenseDataRequest', ) - add_state_id_type( - applicant.state_id_data.state_id_type, - document - ) if IdentityConfig.store.aamva_send_id_type + if IdentityConfig.store.aamva_send_id_type + add_state_id_type( + applicant.state_id_data.state_id_type, + document, + ) + end @body = document.to_s end From 4c3797c605c25a5a8abee1939b45a66ee8345913 Mon Sep 17 00:00:00 2001 From: Matt Wagner Date: Tue, 5 Nov 2024 12:51:02 -0500 Subject: [PATCH 6/7] Fix feature flag to default on, but be off in prod --- config/application.yml.default | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/application.yml.default b/config/application.yml.default index 6b11c393ddf..376fe87c200 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -19,7 +19,7 @@ aamva_auth_url: 'https://example.org:12345/auth/url' aamva_cert_enabled: true aamva_private_key: '' aamva_public_key: '' -aamva_send_id_type: false +aamva_send_id_type: true aamva_supported_jurisdictions: '["AL","AR","AZ","CO","CT","DC","DE","FL","GA","HI","IA","ID","IL","IN","KS","KY","MA","MD","ME","MI","MO","MS","MT","NC","ND","NE","NJ","NM","NV","OH","OR","PA","RI","SC","SD","TN","TX","VA","VT","WA","WI","WV","WY"]' aamva_verification_request_timeout: 5.0 aamva_verification_url: https://example.org:12345/verification/url @@ -502,6 +502,7 @@ development: # production: aamva_auth_url: 'https://authentication-cert.aamva.org/Authentication/Authenticate.svc' + aamva_send_id_type: false aamva_verification_url: 'https://verificationservices-cert.aamva.org:18449/dldv/2.1/online' available_locales: 'en,es,fr' biometric_ial_enabled: false From cac657efca7613e2f35dbca31fa931fc8c25dc6b Mon Sep 17 00:00:00 2001 From: Matt Wagner Date: Tue, 5 Nov 2024 14:59:32 -0500 Subject: [PATCH 7/7] Fix (grossly) test fixture not matching --- .../fixtures/proofing/aamva/requests/verification_request.xml | 4 ++-- .../proofing/aamva/request/verification_request_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/fixtures/proofing/aamva/requests/verification_request.xml b/spec/fixtures/proofing/aamva/requests/verification_request.xml index 32d5a25010f..c42882043d4 100644 --- a/spec/fixtures/proofing/aamva/requests/verification_request.xml +++ b/spec/fixtures/proofing/aamva/requests/verification_request.xml @@ -31,7 +31,7 @@ VA 20176 - + 1 - \ No newline at end of file + diff --git a/spec/services/proofing/aamva/request/verification_request_spec.rb b/spec/services/proofing/aamva/request/verification_request_spec.rb index a4e60c1e987..68c2a874d83 100644 --- a/spec/services/proofing/aamva/request/verification_request_spec.rb +++ b/spec/services/proofing/aamva/request/verification_request_spec.rb @@ -33,7 +33,7 @@ describe '#body' do it 'should be a request body' do - expect(subject.body).to eq(AamvaFixtures.verification_request) + expect(subject.body + "\n").to eq(AamvaFixtures.verification_request) end it 'should escape XML in applicant data' do