Skip to content

Commit

Permalink
LG-5072: Add Start Over to additional steps in the IAL2 flow (#5394)
Browse files Browse the repository at this point in the history
* Repurpose Idv::SessionsController#destroy for IAL2 restart

**Why**: Because not all "Start Over" occurs is expected to occur within the FlowStateMachine (e.g. GPO verification, soon others).

* Add "Start Over" link to IAL2 steps

* Allow have_logged_event with absent attributes

**Why**: Because sometimes events are logged without any attributes, and it's tedious / redundant to have to explicitly pass an empty hash, especially when the original call under test doesn't actually have to pass an empty hash.

* Add Idv::SessionsController spec

* Mock missing view values for IdV phone

start_over_or_cancel uses cancel shared partial, which relies on specific values to determine whether user is signing up

* Mock missing view values for IdV OTP delivery method

* Support logging location params for SessionsController#destroy

* Log "clear and start over" via step, location params

* Opt-in verify_account view for ERB linting

* Remove unused analytics constant

* Remove unnecessary ERBLint exclusion

It passes as-is

* Remove more unnecessary ERBLint exclusions

All passing

* Add trailing comma to _start_over_or_cancel

ERBLint

* Pass step to start over link

**Why**: It's already there!
  • Loading branch information
aduth authored Sep 20, 2021
1 parent 8361af0 commit e8a3798
Show file tree
Hide file tree
Showing 19 changed files with 148 additions and 80 deletions.
22 changes: 20 additions & 2 deletions .erb-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ linters:
- '*/app/views/idv/doc_auth/_error_messages.html.erb'
- '*/app/views/idv/doc_auth/_ssn_init.html.erb'
- '*/app/views/idv/doc_auth/_ssn_update.html.erb'
- '*/app/views/idv/doc_auth/_start_over_or_cancel.html.erb'
- '*/app/views/idv/doc_auth/agreement.html.erb'
- '*/app/views/idv/doc_auth/document_capture.html.erb'
- '*/app/views/idv/doc_auth/link_sent.html.erb'
Expand All @@ -31,7 +30,26 @@ linters:
- '*/app/views/shared/*'
- '*/app/views/two_factor_authentication/*'
- '*/app/views/user_mailer/*'
- '*/app/views/users/*'
- '*/app/views/users/authorization_confirmation/*'
- '*/app/views/users/backup_code_setup/*'
- '*/app/views/users/delete/*'
- '*/app/views/users/edit_phone/*'
- '*/app/views/users/email_language/*'
- '*/app/views/users/emails/*'
- '*/app/views/users/passwords/*'
- '*/app/views/users/phone_setup/*'
- '*/app/views/users/phones/*'
- '*/app/views/users/piv_cac_setup/*'
- '*/app/views/users/piv_cac_setup_from_sign_in/*'
- '*/app/views/users/rules_of_use/*'
- '*/app/views/users/service_provider_inactive/*'
- '*/app/views/users/service_provider_revoke/*'
- '*/app/views/users/shared/*'
- '*/app/views/users/totp_setup/*'
- '*/app/views/users/two_factor_authentication_setup/*'
- '*/app/views/users/verify_password/*'
- '*/app/views/users/verify_personal_key/*'
- '*/app/views/users/webauthn_setup/*'
rubocop_config:
inherit_from:
- .rubocop.yml
Expand Down
15 changes: 13 additions & 2 deletions app/controllers/idv/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,23 @@ class SessionsController < ApplicationController
before_action :confirm_two_factor_authenticated

def destroy
analytics.track_event(Analytics::IDV_VERIFICATION_ATTEMPT_CANCELLED)
Idv::CancelVerificationAttempt.new(user: current_user).call
cancel_verification_attempt_if_pending_profile
analytics.track_event(Analytics::IDV_START_OVER, **location_params)
user_session['idv/doc_auth'] = {}
idv_session.clear
user_session.delete(:decrypted_pii)
redirect_to idv_url
end

private

def cancel_verification_attempt_if_pending_profile
return if current_user.profiles.verification_pending.blank?
Idv::CancelVerificationAttempt.new(user: current_user).call
end

def location_params
params.permit(:step, :location).to_h.symbolize_keys
end
end
end
2 changes: 1 addition & 1 deletion app/services/analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ def browser_attributes
IDV_PHONE_RECORD_VISIT = 'IdV: phone of record visited'.freeze
IDV_REVIEW_COMPLETE = 'IdV: review complete'.freeze
IDV_REVIEW_VISIT = 'IdV: review info visited'.freeze
IDV_START_OVER = 'IdV: start over'.freeze
IDV_GPO_ADDRESS_LETTER_REQUESTED = 'IdV: USPS address letter requested'.freeze
IDV_GPO_ADDRESS_SUBMITTED = 'IdV: USPS address submitted'.freeze
IDV_GPO_ADDRESS_VISITED = 'IdV: USPS address visited'.freeze
IDV_VERIFICATION_ATTEMPT_CANCELLED = 'IdV: verification attempt cancelled'.freeze
INVALID_AUTHENTICITY_TOKEN = 'Invalid Authenticity Token'.freeze
LOGOUT_INITIATED = 'Logout Initiated'.freeze
MULTI_FACTOR_AUTH = 'Multi-Factor Authentication'.freeze
Expand Down
9 changes: 0 additions & 9 deletions app/services/idv/actions/reset_action.rb

This file was deleted.

1 change: 0 additions & 1 deletion app/services/idv/flows/capture_doc_flow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ class CaptureDocFlow < Flow::BaseFlow
].freeze

ACTIONS = {
reset: Idv::Actions::ResetAction,
verify_document: Idv::Actions::VerifyDocumentAction,
verify_document_status: Idv::Actions::VerifyDocumentStatusAction,
}.freeze
Expand Down
1 change: 0 additions & 1 deletion app/services/idv/flows/doc_auth_flow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class DocAuthFlow < Flow::BaseFlow
cancel_send_link: Idv::Actions::CancelSendLinkAction,
cancel_link_sent: Idv::Actions::CancelLinkSentAction,
cancel_update_ssn: Idv::Actions::CancelUpdateSsnAction,
reset: Idv::Actions::ResetAction,
redo_ssn: Idv::Actions::RedoSsnAction,
verify_document: Idv::Actions::VerifyDocumentAction,
verify_document_status: Idv::Actions::VerifyDocumentStatusAction,
Expand Down
7 changes: 4 additions & 3 deletions app/views/idv/doc_auth/_start_over_or_cancel.html.erb
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
<%#
locals:
* hide_start_over: Whether to omit the "Start Over" link
* step: Current step, used in analytics logging.
%>
<div class="margin-top-4">
<% if !local_assigns[:hide_start_over] %>
<%= button_to(
idv_doc_auth_step_path(step: :reset),
method: :put,
class: 'usa-button usa-button--unstyled'
idv_session_path(step: local_assigns[:step]),
method: :delete,
class: 'usa-button usa-button--unstyled',
) { t('doc_auth.buttons.start_over') } %>
<% end %>
<%= render 'shared/cancel', link: idv_cancel_path(step: local_assigns[:step]) %>
Expand Down
18 changes: 2 additions & 16 deletions app/views/idv/doc_auth/agreement.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,6 @@
class: 'usa-button--big usa-button--wide' %>
<% end %>

<br/>
<%= render 'idv/doc_auth/start_over_or_cancel', step: 'agreement' %>


<% if user_fully_authenticated? %>
<%= render 'shared/cancel', link: idv_cancel_path(step: 'agreement') %>
<% else %>
<div class='margin-top-2 padding-top-1 border-top border-primary-light'>
<%= link_to(t('two_factor_authentication.choose_another_option'), two_factor_options_path) %>
</div>
<% end %>


<%= javascript_packs_tag_once(
'clipboard',
'ial2-consent-button',
'document-capture-welcome',
) %>
<%= javascript_packs_tag_once('ial2-consent-button', 'document-capture-welcome') %>
6 changes: 1 addition & 5 deletions app/views/idv/otp_delivery_method/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,4 @@
].select(&:present?),
) %>

<div class="margin-top-4 border-top border-primary-light">
<div class="margin-top-1">
<%= link_to(t('links.cancel'), idv_cancel_path(step: 'phone_otp_delivery_selection')) %>
</div>
</div>
<%= render 'idv/doc_auth/start_over_or_cancel', step: 'phone_otp_delivery_selection' %>
6 changes: 1 addition & 5 deletions app/views/idv/otp_verification/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,4 @@
<%= @presenter.update_phone_link %>
</p>

<div class="margin-top-4 border-top border-primary-light">
<div class="margin-top-1">
<%= link_to t('links.cancel'), idv_cancel_path(step: 'phone_otp_verification') %>
</div>
</div>
<%= render 'idv/doc_auth/start_over_or_cancel', step: 'phone_otp_verification' %>
4 changes: 1 addition & 3 deletions app/views/idv/phone/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@
].select(&:present?),
) %>

<div class="margin-top-2 padding-top-1 border-top border-primary-light">
<%= link_to t('links.cancel'), idv_cancel_path(step: 'phone') %>
</div>
<%= render 'idv/doc_auth/start_over_or_cancel', step: 'phone' %>

<% javascript_packs_tag_once 'form-steps-wait' %>
4 changes: 1 addition & 3 deletions app/views/idv/review/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,4 @@
) %>
<% end %>

<div class="margin-top-6 margin-top-2 padding-top-1 border-top border-primary-light">
<%= link_to t('links.cancel'), idv_cancel_path(step: 'review') %>
</div>
<%= render 'idv/doc_auth/start_over_or_cancel', step: 'review' %>
27 changes: 17 additions & 10 deletions app/views/users/verify_account/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<% content_for(:pre_flash_content) do %>
<%= render 'shared/step_indicator', {
steps: Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS.map do |step|
step[:name] == :secure_account ? step.merge(status: :complete) : step
end,
current_step: :verify_phone_or_address,
locale_scope: 'idv',
class: 'margin-x-neg-2 margin-top-neg-4 tablet:margin-x-neg-6 tablet:margin-top-neg-4',
} %>
steps: Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS.map do |step|
step[:name] == :secure_account ? step.merge(status: :complete) : step
end,
current_step: :verify_phone_or_address,
locale_scope: 'idv',
class: 'margin-x-neg-2 margin-top-neg-4 tablet:margin-x-neg-6 tablet:margin-top-neg-4',
} %>
<% end %>

<% title t('titles.verify_profile') %>
Expand All @@ -17,8 +17,11 @@
<p class="mt-tiny margin-bottom-0">
<%= t('forms.verify_profile.instructions') %>
</p>
<%= validated_form_for(@verify_account_form, url: verify_account_path,
html: { autocomplete: 'off', method: :post }) do |f| %>
<%= validated_form_for(
@verify_account_form,
url: verify_account_path,
html: { autocomplete: 'off', method: :post },
) do |f| %>
<div class="grid-row margin-bottom-5">
<div class="grid-col-12 tablet:grid-col-6">
<%= f.input :otp,
Expand All @@ -43,7 +46,11 @@
<%= link_to t('idv.messages.gpo.resend'), idv_gpo_path, class: 'block margin-bottom-2' %>
<% end %>

<%= button_to(idv_session_path, method: :delete, class: 'usa-button usa-button--unstyled') { t('idv.messages.clear_and_start_over') } %>
<%= button_to(
idv_session_path(step: 'gpo_verify', location: 'clear_and_start_over'),
method: :delete,
class: 'usa-button usa-button--unstyled',
) { t('idv.messages.clear_and_start_over') } %>

<div class="margin-top-2 padding-top-1 border-top border-primary-light">
<%= link_to t('idv.buttons.cancel'), account_path %>
Expand Down
71 changes: 71 additions & 0 deletions spec/controllers/idv/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
require 'rails_helper'

describe Idv::SessionsController do
let(:user) { build(:user) }

before do
stub_sign_in(user)
stub_analytics
end

describe '#destroy' do
let(:idv_session) { double }
let(:flow_session) { {} }
let(:pii) { { first_name: 'Jane' } }

before do
allow(idv_session).to receive(:clear)
allow(subject).to receive(:idv_session).and_return(idv_session)
controller.user_session['idv/doc_auth'] = flow_session
controller.user_session[:decrypted_pii] = pii
end

it 'deletes idv session' do
expect(idv_session).to receive(:clear)

delete :destroy

expect(controller.user_session['idv/doc_auth']).to be_blank
expect(controller.user_session[:decrypted_pii]).to be_blank
end

it 'logs start over with step and location params' do
delete :destroy, params: { step: 'first', location: 'get_help' }

expect(@analytics).to have_logged_event(
Analytics::IDV_START_OVER,
step: 'first',
location: 'get_help',
)
end

it 'redirects to start of identity verificaton' do
delete :destroy

expect(response).to redirect_to(idv_url)
end

context 'pending profile' do
let(:user) do
create(
:user,
profiles: [create(:profile, deactivation_reason: :verification_pending)],
)
end

it 'cancels verification attempt' do
cancel = double
expect(Idv::CancelVerificationAttempt).to receive(:new).and_return(cancel)
expect(cancel).to receive(:call)

delete :destroy, params: { step: 'gpo_verify', location: 'clear_and_start_over' }

expect(@analytics).to have_logged_event(
Analytics::IDV_START_OVER,
step: 'gpo_verify',
location: 'clear_and_start_over',
)
end
end
end
end
19 changes: 0 additions & 19 deletions spec/features/idv/actions/reset_action_spec.rb

This file was deleted.

2 changes: 2 additions & 0 deletions spec/support/fake_analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ def browser_attributes
end

RSpec::Matchers.define :have_logged_event do |event_name, attributes|
attributes ||= {}

match do |actual|
expect(actual).to be_kind_of(FakeAnalytics)

Expand Down
10 changes: 10 additions & 0 deletions spec/views/idv/doc_auth/_start_over_or_cancel.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,14 @@
end
end
end

context 'with step local' do
let(:step) { 'first' }
let(:locals) { { step: step } }

it 'creates links with step parameter' do
expect(subject).to have_link(t('links.cancel', href: idv_cancel_path(step: step)))
expect(subject).to have_css("form[action='#{idv_session_path(step: step)}']")
end
end
end
2 changes: 2 additions & 0 deletions spec/views/idv/otp_delivery_method/new.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
let(:gpo_letter_available) { false }

before do
allow(view).to receive(:user_signing_up?).and_return(false)
allow(view).to receive(:user_fully_authenticated?).and_return(true)
allow(view).to receive(:gpo_letter_available).and_return(gpo_letter_available)
end

Expand Down
2 changes: 2 additions & 0 deletions spec/views/idv/phone/new.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
let(:gpo_letter_available) { false }

before do
allow(view).to receive(:user_signing_up?).and_return(false)
allow(view).to receive(:user_fully_authenticated?).and_return(true)
allow(view).to receive(:gpo_letter_available).and_return(gpo_letter_available)
@idv_form = Idv::PhoneForm.new(user: build_stubbed(:user), previous_params: nil)
end
Expand Down

0 comments on commit e8a3798

Please sign in to comment.