Skip to content

LG-11352: Remove Double Address Verification from job arguments #9829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JackRyan1989 wants to merge 9 commits into from
Closed

LG-11352: Remove Double Address Verification from job arguments #9829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5e90676
Removing DAV from specs
Dec 21, 2023
f28abde
Removing IPP from proofing job, agent, and progressive proofer
Dec 22, 2023
b193d5a
Update adjudicator
Dec 22, 2023
6626899
Fixing comment
Dec 22, 2023
f1c3735
changelog: Internal, In-Person Proofing, Remove DAV vars
Dec 22, 2023
f6f9199
removing trailing commas
Dec 26, 2023
28aa69a
Truly lintfixing removing unneeded arg
Dec 26, 2023
7a2075d
Make perform method spec aware of ipp_enrollment_in_progress
Dec 26, 2023
6c32741
Make agent spec aware of ipp_enrollment_in_progress and liiiint
Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions app/jobs/resolution_proofing_job.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ def perform(
encrypted_arguments:,
trace_id:,
should_proof_state_id:,
double_address_verification: false,
ipp_enrollment_in_progress: false,
ipp_enrollment_in_progress:,
Copy link
Copy Markdown
Contributor

@aduth aduth Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we need to split this over several deploys to gracefully handle 50/50 states?

Ref: https://handbook.login.gov/articles/manage-50-50-state.html#jobs

Copy link
Copy Markdown
Contributor

@svalexander svalexander Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pr is a follow up to this one where ipp_enrollment_in_progress was added in order to eventually replace double_address_verification. This should satisfy the 50/50 requirements correct?

Copy link
Copy Markdown
Contributor

@aduth aduth Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this pull request includes both of Deploy 2 ("Remove the argument in calls to the job") and Deploy 3 ("Remove the argument") from under "Remove an argument from a job #perform method".

Remove the argument in calls to the job: https://github.com/18F/identity-idp/pull/9829/files#diff-b4c1fa063fe7add8eae466de029891f060102bf5c16e4e69e88fe76e5676332fL31

Remove the argument [in the job itself]: https://github.com/18F/identity-idp/pull/9829/files#diff-32ee3d7bfb2561fc0fdef096c00c6533d565fb019e842a72be42b16b9fdaa959L22

Copy link
Copy Markdown
Contributor Author

@JackRyan1989 JackRyan1989 Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah thanks @aduth this PR should be broken into two. @svalexander I didn't quite get it right with the first PR you referenced. That one made sure the jobs are aware of ipp_enrollment_in_progress and that the agent.rb file passes in the correct value to the jobs.

user_id: nil,
threatmetrix_session_id: nil,
request_ip: nil,
Expand All @@ -46,7 +45,6 @@ def perform(
threatmetrix_session_id: threatmetrix_session_id,
request_ip: request_ip,
should_proof_state_id: should_proof_state_id,
double_address_verification: double_address_verification,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
instant_verify_ab_test_discriminator: instant_verify_ab_test_discriminator,
)
Expand Down Expand Up @@ -75,7 +73,6 @@ def make_vendor_proofing_requests(
threatmetrix_session_id:,
request_ip:,
should_proof_state_id:,
double_address_verification:,
ipp_enrollment_in_progress:,
instant_verify_ab_test_discriminator:
)
Expand All @@ -85,7 +82,6 @@ def make_vendor_proofing_requests(
threatmetrix_session_id: threatmetrix_session_id,
request_ip: request_ip,
should_proof_state_id: should_proof_state_id,
double_address_verification: double_address_verification,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
timer: timer,
)
Expand Down
4 changes: 2 additions & 2 deletions app/services/idv/agent.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def proof_resolution(
user_id:,
threatmetrix_session_id:,
request_ip:,
ipp_enrollment_in_progress: false
ipp_enrollment_in_progress:
)
document_capture_session.create_proofing_session

Expand All @@ -28,7 +28,7 @@ def proof_resolution(
user_id: user_id,
threatmetrix_session_id: threatmetrix_session_id,
request_ip: request_ip,
double_address_verification: ipp_enrollment_in_progress,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
}

if IdentityConfig.store.ruby_workers_idv_enabled
Expand Down
44 changes: 11 additions & 33 deletions app/services/proofing/resolution/progressive_proofer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,8 @@ def initialize(instant_verify_ab_test_discriminator = nil)
end

# @param [Hash] applicant_pii keys are symbols and values are strings, confidential user info
# @param [Boolean] double_address_verification flag that indicates if user will have
# both state id address and current residential address verified. Note this value is here as
# a placeholder until it can be replaced with ipp_enrollment_in_progress in ticket LG-353:
# https://cm-jira.usa.gov/browse/LG-11353
# @param [Boolean] ipp_enrollment_in_progress flag that indicates if user will have
Copy link
Copy Markdown
Contributor

@svalexander svalexander Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably still keep this definition in addition to the newly added info that it's used in place of the dav flag

Copy link
Copy Markdown
Contributor Author

@JackRyan1989 JackRyan1989 Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the DAV flag still in place? I thought it was removed from the config?

Copy link
Copy Markdown
Contributor

@svalexander svalexander Jan 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's removed - i was thinking the new comment should be more like: ~ # @param [Boolean] ipp_enrollment_in_progress flag is used in place of DAV flag because DAV is # now always true # it indicates if both state id address and current residential address verified

Copy link
Copy Markdown
Contributor Author

@JackRyan1989 JackRyan1989 Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought ipp_enrollment_in_progress indicates that an enrollment is either in the 'establishing' or 'in progress' statuses? Happy to be wrong though!

Copy link
Copy Markdown
Contributor

@svalexander svalexander Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but it was also meant as a replacement for double_address_verification . So it's like an in person enrollment exists for the user acting as a stand in for the user is going through the ipp flow.

# both state id address and current residential address verified
# @param [Boolean] ipp_enrollment_in_progress flag is used in place of DAV flag because DAV is
# now always true
# @param [String] request_ip IP address for request
# @param [Boolean] should_proof_state_id based on state id jurisdiction, indicates if
# there should be a state id proofing request made to aamva
Expand All @@ -33,8 +29,7 @@ def proof(
should_proof_state_id:,
threatmetrix_session_id:,
timer:,
user_email:,
double_address_verification: false
user_email:
Copy link
Copy Markdown
Contributor

@svalexander svalexander Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we not need ipp_enrollment_in_progress here?

Copy link
Copy Markdown
Contributor Author

@JackRyan1989 JackRyan1989 Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soooo, my open question at the top is more along the lines of "do we need ipp_enrollment_in_progress at all, since I'm pretty sure it's always true? Except when an enrollment is failed or completed it's always in progress or establishing, right?"

)
device_profiling_result = proof_with_threatmetrix_if_needed(
applicant_pii: applicant_pii,
Expand All @@ -47,21 +42,16 @@ def proof(
residential_instant_verify_result = proof_residential_address_if_needed(
applicant_pii: applicant_pii,
timer: timer,
double_address_verification: double_address_verification,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)

applicant_pii_transformed = applicant_pii.clone
if ipp_enrollment_in_progress || double_address_verification
applicant_pii_transformed = with_state_id_address(applicant_pii_transformed)
end
applicant_pii_transformed = with_state_id_address(applicant_pii_transformed)

instant_verify_result = proof_id_address_with_lexis_nexis_if_needed(
applicant_pii: applicant_pii_transformed,
timer: timer,
residential_instant_verify_result: residential_instant_verify_result,
double_address_verification: double_address_verification,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)

state_id_result = proof_id_with_aamva_if_needed(
Expand All @@ -70,13 +60,11 @@ def proof(
residential_instant_verify_result: residential_instant_verify_result,
instant_verify_result: instant_verify_result,
should_proof_state_id: should_proof_state_id,
double_address_verification: double_address_verification,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)

ResultAdjudicator.new(
device_profiling_result: device_profiling_result,
double_address_verification: double_address_verification,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
resolution_result: instant_verify_result,
should_proof_state_id: should_proof_state_id,
Expand Down Expand Up @@ -118,11 +106,9 @@ def proof_with_threatmetrix_if_needed(
def proof_residential_address_if_needed(
applicant_pii:,
timer:,
double_address_verification: false,
ipp_enrollment_in_progress: false
ipp_enrollment_in_progress:
)
return residential_address_unnecessary_result unless
ipp_enrollment_in_progress || double_address_verification
return residential_address_unnecessary_result unless ipp_enrollment_in_progress

timer.time('residential address') do
resolution_proofer.proof(applicant_pii)
Expand All @@ -142,11 +128,8 @@ def resolution_cannot_pass
end

def proof_id_address_with_lexis_nexis_if_needed(applicant_pii:, timer:,
residential_instant_verify_result:,
double_address_verification:,
ipp_enrollment_in_progress:)
if applicant_pii[:same_address_as_id] == 'true' &&
(ipp_enrollment_in_progress || double_address_verification)
residential_instant_verify_result:)
if applicant_pii[:same_address_as_id] == 'true'
return residential_instant_verify_result
end
return resolution_cannot_pass unless residential_instant_verify_result.success?
Expand All @@ -158,14 +141,11 @@ def proof_id_address_with_lexis_nexis_if_needed(applicant_pii:, timer:,

def should_proof_state_id_with_aamva?(ipp_enrollment_in_progress:, same_address_as_id:,
should_proof_state_id:, instant_verify_result:,
residential_instant_verify_result:,
double_address_verification:)
residential_instant_verify_result:)
return false unless should_proof_state_id
# If the user is in double-address-verification and they have changed their address then
# they are not eligible for get-to-yes
# rubocop:disable Layout/LineLength
if !(ipp_enrollment_in_progress || double_address_verification) || same_address_as_id == 'true'
# rubocop:enable Layout/LineLength
if !ipp_enrollment_in_progress || same_address_as_id == 'true'
user_can_pass_after_state_id_check?(instant_verify_result)
else
residential_instant_verify_result.success?
Expand All @@ -177,12 +157,10 @@ def proof_id_with_aamva_if_needed(
residential_instant_verify_result:,
instant_verify_result:,
should_proof_state_id:,
ipp_enrollment_in_progress:,
double_address_verification:
ipp_enrollment_in_progress:
)
same_address_as_id = applicant_pii[:same_address_as_id]
should_proof_state_id_with_aamva = should_proof_state_id_with_aamva?(
double_address_verification:,
ipp_enrollment_in_progress:,
same_address_as_id:,
should_proof_state_id:,
Expand Down
10 changes: 4 additions & 6 deletions app/services/proofing/resolution/result_adjudicator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ module Proofing
module Resolution
class ResultAdjudicator
attr_reader :resolution_result, :state_id_result, :device_profiling_result,
:double_address_verification, :ipp_enrollment_in_progress,
:residential_resolution_result, :same_address_as_id
:ipp_enrollment_in_progress, :residential_resolution_result,
:same_address_as_id

def initialize(
resolution_result:, # InstantVerify
Expand All @@ -12,14 +12,12 @@ def initialize(
should_proof_state_id:,
ipp_enrollment_in_progress:,
device_profiling_result:,
same_address_as_id:,
double_address_verification: false
same_address_as_id:
)
@resolution_result = resolution_result
@state_id_result = state_id_result
@should_proof_state_id = should_proof_state_id
@ipp_enrollment_in_progress = ipp_enrollment_in_progress
@double_address_verification = double_address_verification
@device_profiling_result = device_profiling_result
@residential_resolution_result = residential_resolution_result
@same_address_as_id = same_address_as_id # this is a string, "true" or "false"
Expand Down Expand Up @@ -90,7 +88,7 @@ def device_profiling_result_and_reason

def resolution_result_and_reason
if !residential_resolution_result.success? && same_address_as_id == 'false' &&
(ipp_enrollment_in_progress || double_address_verification)
ipp_enrollment_in_progress
[false, :fail_resolution_skip_state_id]
elsif resolution_result.success? && state_id_result.success?
[true, :pass_resolution_and_state_id]
Expand Down
5 changes: 4 additions & 1 deletion spec/jobs/resolution_proofing_job_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
let(:threatmetrix_session_id) { SecureRandom.uuid }
let(:proofing_device_profiling) { :enabled }
let(:lexisnexis_threatmetrix_mock_enabled) { false }
let(:ipp_enrollment_in_progress) { true }

before do
allow(IdentityConfig.store).to receive(:proofing_device_profiling).
Expand All @@ -40,6 +41,7 @@
user_id: user.id,
threatmetrix_session_id: threatmetrix_session_id,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)
end

Expand Down Expand Up @@ -118,6 +120,7 @@
user_id: user.id,
threatmetrix_session_id: threatmetrix_session_id,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)
end
it 'stores a successful result' do
Expand Down Expand Up @@ -411,7 +414,7 @@
user_id: user.id,
threatmetrix_session_id: threatmetrix_session_id,
request_ip: request_ip,
ipp_enrollment_in_progress: true,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)
end

Expand Down
7 changes: 7 additions & 0 deletions spec/services/idv/agent_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
let(:issuer) { 'fake-issuer' }
let(:friendly_name) { 'fake-name' }
let(:app_id) { 'fake-app-id' }
let(:ipp_enrollment_in_progress) { true }

let(:agent) { Idv::Agent.new(applicant) }

Expand All @@ -41,6 +42,7 @@
user_id: user.id,
threatmetrix_session_id: nil,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)

result = document_capture_session.load_proofing_result.result
Expand All @@ -57,6 +59,7 @@
user_id: user.id,
threatmetrix_session_id: nil,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)
result = document_capture_session.load_proofing_result.result
expect(result[:context][:stages][:state_id]).to include(
Expand All @@ -82,6 +85,7 @@
user_id: user.id,
threatmetrix_session_id: nil,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)
result = document_capture_session.load_proofing_result.result
expect(result[:errors][:ssn]).to eq ['Unverified SSN.']
Expand All @@ -97,6 +101,7 @@
user_id: user.id,
threatmetrix_session_id: nil,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)

result = document_capture_session.load_proofing_result.result
Expand All @@ -118,6 +123,7 @@
user_id: user.id,
threatmetrix_session_id: nil,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)
result = document_capture_session.load_proofing_result.result

Expand All @@ -142,6 +148,7 @@
user_id: user.id,
threatmetrix_session_id: nil,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
)
result = document_capture_session.load_proofing_result.result

Expand Down
4 changes: 0 additions & 4 deletions spec/services/proofing/resolution/progressive_proofer_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
let(:applicant_pii) { Idp::Constants::MOCK_IDV_APPLICANT_STATE_ID_ADDRESS }
let(:should_proof_state_id) { true }
let(:ipp_enrollment_in_progress) { true }
let(:double_address_verification) { true }
let(:request_ip) { Faker::Internet.ip_v4_address }
let(:threatmetrix_session_id) { SecureRandom.uuid }
let(:timer) { JobHelpers::Timer.new }
Expand Down Expand Up @@ -42,7 +41,6 @@
instance.proof(
applicant_pii: applicant_pii,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
double_address_verification: double_address_verification,
request_ip: request_ip,
should_proof_state_id: should_proof_state_id,
threatmetrix_session_id: threatmetrix_session_id,
Expand Down Expand Up @@ -180,7 +178,6 @@

expect(result.same_address_as_id).to eq('true')
expect(result.ipp_enrollment_in_progress).to eq(true)
expect(result.double_address_verification).to eq(true)
expect(result.resolution_result).to eq(result.residential_resolution_result)
end

Expand Down Expand Up @@ -299,7 +296,6 @@
instance_double(Proofing::Resolution::Result)
end
let(:ipp_enrollment_in_progress) { true }
let(:double_address_verification) { true }
let(:applicant_pii) do
JSON.parse(<<-STR, symbolize_names: true)
{
Expand Down
29 changes: 0 additions & 29 deletions spec/services/proofing/resolution/result_adjudicator_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
end

let(:should_proof_state_id) { true }
let(:double_address_verification) { true }
let(:ipp_enrollment_in_progress) { true }
let(:same_address_as_id) { 'false' }

Expand All @@ -52,7 +51,6 @@
state_id_result: state_id_result,
should_proof_state_id: should_proof_state_id,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
double_address_verification: double_address_verification,
device_profiling_result: device_profiling_result,
same_address_as_id: same_address_as_id,
)
Expand Down Expand Up @@ -93,33 +91,6 @@
expect(resolution_adjudication_reason).to eq(:fail_state_id)
end
end

# rubocop:disable Layout/LineLength
context 'Confirm adjudication works for either double_address_verification or ipp_enrollment_in_progress' do
context 'Adjudication passes if double_address_verification is false and ipp_enrollment_in_progress is true' do
# rubocop:enable Layout/LineLength
let(:double_address_verification) { false }
let(:ipp_enrollment_in_progress) { true }

it 'returns a successful response' do
result = subject.adjudicated_result

expect(result.success?).to eq(true)
end
end
# rubocop:disable Layout/LineLength
context 'Adjudication passes if ipp_enrollment_in_progress is false and double_address_verification is true' do
# rubocop:enable Layout/LineLength
let(:double_address_verification) { true }
let(:ipp_enrollment_in_progress) { false }

it 'returns a successful response' do
result = subject.adjudicated_result

expect(result.success?).to eq(true)
end
end
end
end
end
end