Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion app/controllers/idv/enter_password_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def self.step_info
key: :enter_password,
controller: self,
action: :new,
next_steps: [FlowPolicy::FINAL],
next_steps: [:personal_key],
preconditions: ->(idv_session:, user:) do
idv_session.phone_or_address_step_complete?
end,
Expand Down
46 changes: 26 additions & 20 deletions app/controllers/idv/personal_key_controller.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
module Idv
class PersonalKeyController < ApplicationController
include Idv::AvailabilityConcern
include IdvSession
include IdvStepConcern
include StepIndicatorConcern
include SecureHeadersConcern
include FraudReviewConcern
include OptInHelper

before_action :apply_secure_headers_override
before_action :confirm_two_factor_authenticated
before_action :confirm_phone_or_address_confirmed
before_action :confirm_profile_has_been_created
before_action :confirm_personal_key_not_acknowledged
before_action :confirm_step_allowed

# Personal key is kind of a special case, since you're always meant to
# look at it after your profile has been minted. We opt out of a few
# standard before_actions and handle them in our own special way below.
skip_before_action :confirm_idv_needed
skip_before_action :confirm_personal_key_acknowledged_if_needed
skip_before_action :confirm_no_pending_in_person_enrollment
skip_before_action :handle_fraud

def show
analytics.idv_personal_key_visited(
Expand All @@ -38,6 +42,22 @@ def update
redirect_to next_step
end

def self.step_info
Idv::StepInfo.new(
key: :personal_key,
controller: self,
next_steps: [FlowPolicy::FINAL],
preconditions: ->(idv_session:, user:) do
idv_session.phone_or_address_step_complete? &&
user.active_or_pending_profile &&
!idv_session.personal_key_acknowledged
end,
undo_step: ->(idv_session:, user:) {
idv_session.invalidate_personal_key!
},
)
end

private

def next_step
Expand All @@ -52,20 +72,6 @@ def next_step
end
end

def confirm_phone_or_address_confirmed
return if idv_session.address_confirmed? || idv_session.phone_confirmed?

redirect_to idv_enter_password_url
end

def confirm_personal_key_not_acknowledged
redirect_to next_step if idv_session.personal_key_acknowledged
end

def confirm_profile_has_been_created
redirect_to account_url if profile.blank?
end

def add_proofing_component
ProofingComponent.find_or_create_by(user: current_user).update(verified_at: Time.zone.now)
end
Expand Down
1 change: 1 addition & 0 deletions app/policies/idv/flow_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ def steps
otp_verification: Idv::OtpVerificationController.step_info,
request_letter: Idv::ByMail::RequestLetterController.step_info,
enter_password: Idv::EnterPasswordController.step_info,
personal_key: Idv::PersonalKeyController.step_info,
}
end

Expand Down
208 changes: 165 additions & 43 deletions spec/controllers/idv/personal_key_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require 'rails_helper'

RSpec.describe Idv::PersonalKeyController do
include FlowPolicyHelper
include SamlAuthHelper
include PersonalKeyValidator

Expand Down Expand Up @@ -42,91 +43,198 @@ def assert_personal_key_generated_for_profiles(*profile_pii_pairs)

let(:idv_session) { subject.idv_session }

let(:threatmetrix_review_status) { nil }

before do
stub_analytics
stub_attempts_tracker
stub_verify_steps_one_and_two(user, applicant: applicant)

stub_sign_in(user)

case address_verification_mechanism
when 'phone'
idv_session.address_verification_mechanism = 'phone'
idv_session.user_phone_confirmation = true
idv_session.vendor_phone_confirmation = true
stub_up_to(:otp_verification, idv_session: idv_session)
when 'gpo'
idv_session.address_verification_mechanism = 'gpo'
idv_session.user_phone_confirmation = false
idv_session.vendor_phone_confirmation = false
stub_up_to(:request_letter, idv_session: idv_session)
idv_session.gpo_code_verified = true
when nil
stub_up_to(:verify_info, idv_session: idv_session)
else
raise 'invalid address_verification_mechanism'
end

idv_session.applicant = applicant

if mint_profile_from_idv_session
idv_session.create_profile_from_applicant_with_password(password)
end
end

describe '#step_info' do
let(:step_info) do
controller.class.step_info
end

describe '#undo_step' do
it 'clears personal_key_acknowledged' do
idv_session.acknowledge_personal_key!
step_info.undo_step.call(idv_session: idv_session, user: user)
expect(idv_session.personal_key_acknowledged).to eql(nil)
end

it 'clears personal_key' do
idv_session.personal_key = 'ABCD-1234'
step_info.undo_step.call(idv_session: idv_session, user: user)
expect(idv_session.personal_key).to be_nil
end
end

describe '#preconditions' do
Comment thread
matthinz marked this conversation as resolved.
let(:preconditions) do
step_info.preconditions.call(idv_session: idv_session, user: user)
end

context 'when all conditions met' do
it 'returns a truthy result' do
expect(preconditions).to be_truthy
end
end

context 'when user does not have a pending or active profile' do
before do
user.active_profile.deactivate(:password_reset)
expect(user.active_profile).to eql(nil)
user.reload
end

it 'returns something falsey' do
expect(preconditions).to be_falsey
end
end

context 'when address confirmed via GPO' do
let(:address_verification_mechanism) { 'gpo' }
it 'returns a truthy result' do
expect(preconditions).to be_truthy
end
end

context 'when address confirmed via phone' do
let(:address_verification_mechanism) { 'phone' }
it 'returns a truthy result' do
expect(preconditions).to be_truthy
end
end

context 'when address unconfirmed' do
let(:address_verification_mechanism) { nil }
it 'returns a falsey result' do
expect(preconditions).to be_falsey
end
end

context 'when personal_key_acknowledged is false' do
before do
idv_session.personal_key_acknowledged = false
end
it 'returns a truthy result' do
expect(preconditions).to be_truthy
end
end

context 'when personal_key_acknowledged is true' do
before do
idv_session.personal_key_acknowledged = true
end
it 'returns a falsey result' do
expect(preconditions).to be_falsey
end
end

context 'when personal_key_acknowledged is nil' do
before do
idv_session.personal_key_acknowledged = nil
end
it 'returns a truthy result' do
expect(preconditions).to be_truthy
end
end
end
end

describe 'before_actions' do
it 'includes before_actions' do
expect(subject).to have_actions(
:before,
:confirm_two_factor_authenticated,
:confirm_phone_or_address_confirmed,
:confirm_step_allowed,
)
end

it 'skips redundant or irrelevant before_actions' do
expect(subject).not_to have_actions(
:before,
:confirm_idv_needed,
:confirm_personal_key_acknowledged_if_needed,
:confirm_no_pending_in_person_enrollment,
:handle_fraud,
)
end

it 'includes before_actions from IdvSession' do
expect(subject).to have_actions(:before, :redirect_unless_sp_requested_verification)
expect(subject).to have_actions(
:before,
:redirect_unless_sp_requested_verification,
)
end
end

describe '#confirm_profile_has_been_created' do
controller do
before_action :confirm_profile_has_been_created
describe '#show' do
context 'profile has been created from idv_session' do
it 'does not redirect' do
get :show

def index
render plain: 'Hello'
end
expect(response).to_not be_redirect
end

context 'profile has been created' do
context 'profile is pending fraud review' do
let(:threatmetrix_review_status) { 'reject' }
it 'does not redirect' do
get :index

get :show
expect(response).to_not be_redirect
end
end
end

context 'profile has not been created from idv_session' do
let(:mint_profile_from_idv_session) { false }
context 'profile has not been created from idv_session' do
let(:mint_profile_from_idv_session) { false }

it 'redirects to the account path' do
get :index
expect(response).to redirect_to account_path
end
it 'redirects to the enter password screen' do
get :show
expect(response).to redirect_to idv_enter_password_url
end

context 'profile is pending from a different session' do
context 'profile is pending due to fraud review' do
let!(:pending_profile) { create(:profile, :fraud_review_pending, user: user) }
context 'but a profile is pending from a different session' do
context 'due to fraud review' do
let!(:pending_profile) { create(:profile, :fraud_review_pending, user: user) }

it 'does not redirect' do
get :index
expect(response).to_not be_redirect
end
it 'does not redirect' do
get :show
expect(response).not_to be_redirect
end
end

context 'profile is pending due to in person proofing' do
let!(:pending_profile) { create(:profile, :in_person_verification_pending, user: user) }
context 'due to in person proofing' do
let!(:pending_profile) { create(:profile, :in_person_verification_pending, user: user) }

it 'does not redirect' do
get :index
expect(response).to_not be_redirect
end
it 'does not redirect' do
get :show
expect(response).to_not be_redirect
end
end
end
end
end

describe '#show' do
it 'sets code instance variable' do
code = idv_session.personal_key
expect(code).to be_present
Expand Down Expand Up @@ -165,10 +273,10 @@ def index
context 'user selected gpo verification' do
let(:address_verification_mechanism) { 'gpo' }

it 'redirects to enter password url' do
it 'redirects to letter enqueued url' do
get :show

expect(response).to redirect_to idv_enter_password_url
expect(response).to redirect_to idv_letter_enqueued_url
end
end

Expand Down Expand Up @@ -266,6 +374,16 @@ def index
end
end
end

context 'personal key already acknowledged' do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As noted above, I think this is a back button prevention check and it should just show the personal key rather than redirecting. Especially since the user might realize they didn't save their key after all and want to go back and look at it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think you may be right

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thinking more about it, this would be a change from how we operate right now--we explicitly remove the personal key from the session after it's been acknowledged. So I think showing the basic "Your identity has already been verified" for now is the right call and I can capture the change in a follow up ticket

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you have a chance to put in this followup ticket?

before do
idv_session.acknowledge_personal_key!
end
it 'redirects away' do
get :show
expect(response).to be_redirect
end
end
end

describe '#update' do
Expand Down Expand Up @@ -307,10 +425,14 @@ def index
context 'user selected gpo verification' do
let(:address_verification_mechanism) { 'gpo' }

it 'redirects to review url' do
it 'redirects to correct url' do
patch :update
expect(response).to redirect_to idv_letter_enqueued_url
end

expect(response).to redirect_to idv_enter_password_url
it 'does not log any events' do
expect(@analytics).not_to have_logged_event
patch :update
end
end

Expand Down
Loading