From 1feaaebd077a84e0a64cccafa8811de4fdc74742 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Oct 2020 15:46:20 -0400 Subject: [PATCH 1/3] LG-3423: Refactor doc capture (hybrid) polling to own endpoint **Why**: Isolate standalone polling behavior for easier disambiguation of controller as exclusively responding to JSON, improved ability to implement test specs for controller in isolation. --- .../idv/capture_doc_status_controller.rb | 41 +++++ app/controllers/idv/doc_auth_controller.rb | 28 ---- app/javascript/packs/doc_capture_polling.js | 2 +- config/routes.rb | 2 +- .../idv/capture_doc_status_controller_spec.rb | 147 ++++++++++++++++++ 5 files changed, 190 insertions(+), 30 deletions(-) create mode 100644 app/controllers/idv/capture_doc_status_controller.rb create mode 100644 spec/controllers/idv/capture_doc_status_controller_spec.rb diff --git a/app/controllers/idv/capture_doc_status_controller.rb b/app/controllers/idv/capture_doc_status_controller.rb new file mode 100644 index 00000000000..db2ac6f907c --- /dev/null +++ b/app/controllers/idv/capture_doc_status_controller.rb @@ -0,0 +1,41 @@ +module Idv + class CaptureDocStatusController < ApplicationController + before_action :confirm_two_factor_authenticated + + respond_to :json + + def show + result = if FeatureManagement.document_capture_step_enabled? + document_capture_session_poll_render_result + else + doc_capture_poll_render_result + end + + render result + end + + private + + def doc_capture_poll_render_result + doc_capture = DocCapture.find_by(user_id: current_user.id) + return { plain: 'Unauthorized', status: :unauthorized } if doc_capture.blank? + return { plain: 'Pending', status: :accepted } if doc_capture.acuant_token.blank? + { plain: 'Complete', status: :ok } + end + + def document_capture_session_poll_render_result + session_uuid = flow_session[:document_capture_session_uuid] + document_capture_session = DocumentCaptureSession.find_by(uuid: session_uuid) + return { plain: 'Unauthorized', status: :unauthorized } unless document_capture_session + + result = document_capture_session.load_result + return { plain: 'Pending', status: :accepted } if result.blank? + return { plain: 'Unauthorized', status: :unauthorized } unless result.success? + { plain: 'Complete', status: :ok } + end + + def flow_session + user_session['idv/doc_auth'] + end + end +end diff --git a/app/controllers/idv/doc_auth_controller.rb b/app/controllers/idv/doc_auth_controller.rb index dbabca0d0a5..ce1202b2cc3 100644 --- a/app/controllers/idv/doc_auth_controller.rb +++ b/app/controllers/idv/doc_auth_controller.rb @@ -16,34 +16,6 @@ class DocAuthController < ApplicationController analytics_id: Analytics::DOC_AUTH, }.freeze - def doc_capture_poll - result = if FeatureManagement.document_capture_step_enabled? - document_capture_session_poll_render_result - else - doc_capture_poll_render_result - end - - render result - end - - def doc_capture_poll_render_result - doc_capture = DocCapture.find_by(user_id: user_id) - return { plain: 'Not authorized', status: :not_authorized } if doc_capture.blank? - return { plain: 'Pending', status: :accepted } if doc_capture.acuant_token.blank? - { plain: 'Complete', status: :ok } - end - - def document_capture_session_poll_render_result - session_uuid = flow_session[:document_capture_session_uuid] - document_capture_session = DocumentCaptureSession.find_by(uuid: session_uuid) - return { plain: 'Not authorized', status: :not_authorized } unless document_capture_session - - result = document_capture_session.load_result - return { plain: 'Pending', status: :accepted } if result.blank? - return { plain: 'Not authorized', status: :not_authorized } unless result.success? - { plain: 'Complete', status: :ok } - end - def redirect_if_mail_bounced redirect_to idv_usps_url if current_user.decorate.usps_mail_bounced? end diff --git a/app/javascript/packs/doc_capture_polling.js b/app/javascript/packs/doc_capture_polling.js index 0b96341ab46..d9e4cf4b294 100644 --- a/app/javascript/packs/doc_capture_polling.js +++ b/app/javascript/packs/doc_capture_polling.js @@ -20,7 +20,7 @@ const docCaptureComplete = () => { const sendDocAuthPollRequest = () => { const request = new XMLHttpRequest(); - request.open('GET', '/verify/doc_auth/link_sent/poll/', true); + request.open('GET', '/verify/capture_doc_status/', true); request.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); request.onload = function () { // This endpoint renders a 202 for a pending request diff --git a/config/routes.rb b/config/routes.rb index cf5553d9b2f..46f0d932f92 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -317,13 +317,13 @@ get '/doc_auth' => 'doc_auth#index' get '/doc_auth/:step' => 'doc_auth#show', as: :doc_auth_step put '/doc_auth/:step' => 'doc_auth#update' - get '/doc_auth/link_sent/poll' => 'doc_auth#doc_capture_poll' get '/capture_doc' => 'capture_doc#index' get '/capture-doc' => 'capture_doc#index', # sometimes underscores get messed up when linked to via SMS as: :capture_doc_dashes get '/capture_doc/:step' => 'capture_doc#show', as: :capture_doc_step put '/capture_doc/:step' => 'capture_doc#update' + get '/capture_doc_status' => 'capture_doc_status#show' unless FeatureManagement.disallow_ial2_recovery? get '/recovery' => 'recovery#index' get '/recovery/:step' => 'recovery#show', as: :recovery_step diff --git a/spec/controllers/idv/capture_doc_status_controller_spec.rb b/spec/controllers/idv/capture_doc_status_controller_spec.rb new file mode 100644 index 00000000000..b2f8c09e301 --- /dev/null +++ b/spec/controllers/idv/capture_doc_status_controller_spec.rb @@ -0,0 +1,147 @@ +require 'rails_helper' + +describe Idv::CaptureDocStatusController do + let(:user) { build(:user) } + let(:document_capture_step_enabled) { false } + + before do |example| + stub_sign_in(user) unless example.metadata[:skip_sign_in] + + allow(FeatureManagement).to receive(:document_capture_step_enabled?). + and_return(document_capture_step_enabled) + end + + describe '#show' do + context 'when unauthenticated', :skip_sign_in do + it 'redirects to the root url' do + get :show + + expect(response).to redirect_to root_url + end + end + + context 'when document capture step is disabled' do + let(:document_capture_step_enabled) { false } + let(:doc_capture) { nil } + + before do + allow(DocCapture).to receive(:find_by).and_return(doc_capture) + end + + context 'when session does not exist' do + let(:doc_capture) { nil } + + it 'returns unauthorized' do + get :show + + expect(response.status).to eq(401) + expect(response.body).to eq('Unauthorized') + end + end + + context 'when result is pending' do + let(:doc_capture) do + DocCapture.create( + user_id: user.id, + request_token: SecureRandom.uuid, + requested_at: Time.zone.now, + ) + end + + it 'returns pending result' do + get :show + + expect(response.status).to eq(202) + expect(response.body).to eq('Pending') + end + end + + context 'when capture is complete' do + let(:doc_capture) do + DocCapture.create( + user_id: user.id, + request_token: SecureRandom.uuid, + requested_at: Time.zone.now, + acuant_token: SecureRandom.uuid, + ) + end + + it 'returns success' do + get :show + + expect(response.status).to eq(200) + expect(response.body).to eq('Complete') + end + end + end + + context 'when document capture step is enabled' do + let(:document_capture_step_enabled) { true } + let(:document_capture_session) { DocumentCaptureSession.create! } + let(:flow_session) { { document_capture_session_uuid: document_capture_session.uuid } } + + before do + allow_any_instance_of(Flow::BaseFlow).to receive(:flow_session).and_return(flow_session) + controller.user_session['idv/doc_auth'] = flow_session + end + + context 'when session does not exist' do + let(:flow_session) { {} } + + it 'returns unauthorized' do + get :show + + expect(response.status).to eq(401) + expect(response.body).to eq('Unauthorized') + end + end + + context 'when result is pending' do + it 'returns pending result' do + get :show + + expect(response.status).to eq(202) + expect(response.body).to eq('Pending') + end + end + + context 'when capture failed' do + before do + allow(EncryptedRedisStructStorage).to receive(:load).and_return( + DocumentCaptureSessionResult.new( + id: SecureRandom.uuid, + success: false, + pii: {}, + ), + ) + end + + it 'returns unauthorized' do + get :show + + expect(response.status).to eq(401) + expect(response.body).to eq('Unauthorized') + end + end + + context 'when capture succeeded' do + before do + allow(EncryptedRedisStructStorage).to receive(:load).and_return( + DocumentCaptureSessionResult.new( + id: SecureRandom.uuid, + success: true, + pii: {}, + ), + ) + end + + it 'returns success' do + get :show + + expect(response.status).to eq(200) + expect(response.body).to eq('Complete') + end + end + end + end +end From 8e644ddf6748a83731bace59ac2ca4ab1b245062 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Oct 2020 16:31:18 -0400 Subject: [PATCH 2/3] Revert capture doc status URL to original path See: https://github.com/18F/identity-idp/pull/4293#discussion_r501976966 --- app/javascript/packs/doc_capture_polling.js | 2 +- config/routes.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/packs/doc_capture_polling.js b/app/javascript/packs/doc_capture_polling.js index d9e4cf4b294..0b96341ab46 100644 --- a/app/javascript/packs/doc_capture_polling.js +++ b/app/javascript/packs/doc_capture_polling.js @@ -20,7 +20,7 @@ const docCaptureComplete = () => { const sendDocAuthPollRequest = () => { const request = new XMLHttpRequest(); - request.open('GET', '/verify/capture_doc_status/', true); + request.open('GET', '/verify/doc_auth/link_sent/poll/', true); request.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); request.onload = function () { // This endpoint renders a 202 for a pending request diff --git a/config/routes.rb b/config/routes.rb index 46f0d932f92..c7037351dec 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -317,13 +317,13 @@ get '/doc_auth' => 'doc_auth#index' get '/doc_auth/:step' => 'doc_auth#show', as: :doc_auth_step put '/doc_auth/:step' => 'doc_auth#update' + get '/doc_auth/link_sent/poll' => 'capture_doc_status#show' get '/capture_doc' => 'capture_doc#index' get '/capture-doc' => 'capture_doc#index', # sometimes underscores get messed up when linked to via SMS as: :capture_doc_dashes get '/capture_doc/:step' => 'capture_doc#show', as: :capture_doc_step put '/capture_doc/:step' => 'capture_doc#update' - get '/capture_doc_status' => 'capture_doc_status#show' unless FeatureManagement.disallow_ial2_recovery? get '/recovery' => 'recovery#index' get '/recovery/:step' => 'recovery#show', as: :recovery_step From b573ed58df9be7d904789eb1a5d45d267b924b01 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 8 Oct 2020 16:34:14 -0400 Subject: [PATCH 3/3] Use existing user helper value for stubbed sign in --- .../controllers/idv/capture_doc_status_controller_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/controllers/idv/capture_doc_status_controller_spec.rb b/spec/controllers/idv/capture_doc_status_controller_spec.rb index b2f8c09e301..28d8fac3f3c 100644 --- a/spec/controllers/idv/capture_doc_status_controller_spec.rb +++ b/spec/controllers/idv/capture_doc_status_controller_spec.rb @@ -4,15 +4,17 @@ let(:user) { build(:user) } let(:document_capture_step_enabled) { false } - before do |example| - stub_sign_in(user) unless example.metadata[:skip_sign_in] + before do + stub_sign_in(user) if user allow(FeatureManagement).to receive(:document_capture_step_enabled?). and_return(document_capture_step_enabled) end describe '#show' do - context 'when unauthenticated', :skip_sign_in do + context 'when unauthenticated' do + let(:user) { nil } + it 'redirects to the root url' do get :show