From a76776bc7d7f6cac446390092310d4a7848adfee Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Tue, 7 Feb 2023 13:46:01 -0500 Subject: [PATCH] Check if a user is authenticate before checking whether they have completed the verify info step Currently we have some code in place to check that a user has completed the verify step in the review controller. This code supports us when both the FSM verify step and the new verify step are in place. It will be removed in this change request: https://github.com/18F/identity-idp/pull/7747 After deploying the IDP with the feature flag enabled we discovered a bug in this code. This code calls `idv_session` in a before action that is added prior to adding the `IdvSession` concern. The `IdvSessnion` concern adds a before action to confirm the user is authenticated. This concern includes a before action to confirm the user is authenticated since calling `idv_session` with no user session results in a `NoMethodError`. Having the `confirm_verify_info_complete` prior to that before action is problematic since it calls `idv_session`. This commit works around the issue by guarding the before action with a check to confirm the user is authenticated. This is not the most elegant solution, but it should work and as stated previously will be removed in #7747. [skip changelog] --- app/controllers/idv/review_controller.rb | 9 ++++--- .../controllers/idv/review_controller_spec.rb | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/app/controllers/idv/review_controller.rb b/app/controllers/idv/review_controller.rb index 200e6758708..de9a83f6c5c 100644 --- a/app/controllers/idv/review_controller.rb +++ b/app/controllers/idv/review_controller.rb @@ -126,10 +126,11 @@ def password end def confirm_verify_info_complete - if IdentityConfig.store.doc_auth_verify_info_controller_enabled && - !idv_session.resolution_successful - redirect_to idv_verify_info_url - end + return unless IdentityConfig.store.doc_auth_verify_info_controller_enabled + return unless user_fully_authenticated? + return if idv_session.resolution_successful + + redirect_to idv_verify_info_url end def personal_key_confirmed diff --git a/spec/controllers/idv/review_controller_spec.rb b/spec/controllers/idv/review_controller_spec.rb index 710ee00625b..e8343696ff0 100644 --- a/spec/controllers/idv/review_controller_spec.rb +++ b/spec/controllers/idv/review_controller_spec.rb @@ -251,6 +251,32 @@ def show expect(flash.now[:success]).to be_nil end end + + context 'doc_auth_verify_info_controller_enabled is set to true' do + before do + allow(IdentityConfig.store).to receive(:doc_auth_verify_info_controller_enabled). + and_return(true) + end + + it 'redirects to the verify info controller if the user has not completed it' do + controller.idv_session.resolution_successful = nil + + get :new + + expect(response).to redirect_to(idv_verify_info_url) + end + + it 'redirects to the root if the user is not authenticated' do + allow(controller).to receive(:user_fully_authenticated?).and_return(false) + allow(controller).to receive(:user_session).and_call_original + allow(controller).to receive(:confirm_two_factor_authenticated).and_call_original + allow(controller).to receive(:current_user).and_call_original + + get :new + + expect(response).to redirect_to(root_url) + end + end end describe '#create' do