diff --git a/app/controllers/idv/image_uploads_controller.rb b/app/controllers/idv/image_uploads_controller.rb index acec62f4a9f..44c0b04afc2 100644 --- a/app/controllers/idv/image_uploads_controller.rb +++ b/app/controllers/idv/image_uploads_controller.rb @@ -10,19 +10,26 @@ def create form_response = image_form.submit if form_response.success? - doc_response = doc_auth_client.post_images( + client_response = doc_auth_client.post_images( front_image: image_form.front.read, back_image: image_form.back.read, selfie_image: image_form.selfie&.read, liveness_checking_enabled: liveness_checking_enabled?, ) - store_pii(doc_response) if doc_response.success? - - render_form_response(doc_response) + store_pii(client_response) if client_response.success? + status = :bad_request unless client_response.success? else - render_form_response(form_response) + status = image_form.status end + + presenter = ImageUploadResponsePresenter.new( + form: image_form, + form_response: client_response || form_response, + ) + + render json: presenter, + status: status || :ok end private @@ -42,21 +49,6 @@ def store_pii(doc_response) image_form.document_capture_session.store_result_from_response(doc_response) end - def render_form_response(form_response) - if form_response.success? - render json: { - success: true, - } - else - errors = form_response.errors.flat_map do |key, errs| - Array(errs).map { |err| { field: key, message: err } } - end - - render json: form_response.to_h.merge(errors: errors), - status: :bad_request - end - end - def doc_auth_client @doc_auth_client ||= DocAuth::Client.client end diff --git a/app/forms/idv/api_image_upload_form.rb b/app/forms/idv/api_image_upload_form.rb index 1f64925569d..ed76eb80c2c 100644 --- a/app/forms/idv/api_image_upload_form.rb +++ b/app/forms/idv/api_image_upload_form.rb @@ -15,6 +15,7 @@ class ApiImageUploadForm validates_presence_of :selfie, if: :liveness_checking_enabled? validate :validate_images + validate :throttle_if_rate_limited def initialize(params, liveness_checking_enabled:) @params = params @@ -22,13 +23,27 @@ def initialize(params, liveness_checking_enabled:) end def submit + throttled_else_increment + FormResponse.new( success: valid?, errors: errors.messages, - extra: {}, + extra: { + remaining_attempts: remaining_attempts, + }, ) end + def status + return :ok if valid? + return :too_many_requests if errors.key?(:limit) + :bad_request + end + + def remaining_attempts + Throttler::RemainingCount.call(document_capture_session.user_id, :idv_acuant) + end + def liveness_checking_enabled? @liveness_checking_enabled end @@ -66,6 +81,18 @@ def self.human_attribute_name(attr, options = {}) attr_reader :params + def throttle_if_rate_limited + return unless @throttled + errors.add(:limit, t('errors.doc_auth.acuant_throttle')) + end + + def throttled_else_increment + @throttled = Throttler::IsThrottledElseIncrement.call( + document_capture_session.user_id, + :idv_acuant, + ) + end + def validate_images IMAGE_KEYS.each do |image_key| validate_image(image_key) if params[image_key] diff --git a/app/presenters/image_upload_response_presenter.rb b/app/presenters/image_upload_response_presenter.rb new file mode 100644 index 00000000000..b854b38ccb5 --- /dev/null +++ b/app/presenters/image_upload_response_presenter.rb @@ -0,0 +1,28 @@ +class ImageUploadResponsePresenter + def initialize(form:, form_response:) + @form = form + @form_response = form_response + end + + def success + @form_response.success? + end + + def errors + @form_response.errors.flat_map do |key, errs| + Array(errs).map { |err| { field: key, message: err } } + end + end + + def remaining_attempts + @form.remaining_attempts + end + + def as_json(*) + { + success: success, + errors: errors, + remaining_attempts: remaining_attempts, + } + end +end diff --git a/spec/controllers/idv/image_uploads_controller_spec.rb b/spec/controllers/idv/image_uploads_controller_spec.rb index 0ff0b00eb1a..f0aaf0c883b 100644 --- a/spec/controllers/idv/image_uploads_controller_spec.rb +++ b/spec/controllers/idv/image_uploads_controller_spec.rb @@ -42,6 +42,7 @@ action json = JSON.parse(response.body, symbolize_names: true) + expect(response.status).to eq(400) expect(json[:success]).to eq(false) expect(json[:errors]).to eq [ { field: 'front', message: 'Please fill in this field.' }, @@ -56,6 +57,7 @@ action json = JSON.parse(response.body, symbolize_names: true) + expect(response.status).to eq(400) expect(json[:errors]).to eq [ { field: 'front', message: I18n.t('doc_auth.errors.not_a_file') }, ] @@ -68,6 +70,7 @@ action json = JSON.parse(response.body, symbolize_names: true) + expect(response.status).to eq(400) expect(json[:errors]).to eq [ { field: 'front', message: I18n.t('doc_auth.errors.not_a_file', locale: 'es') }, ] @@ -75,13 +78,48 @@ end end + context 'throttling' do + it 'returns remaining_attempts with error' do + params.delete(:front) + allow(Throttler::RemainingCount).to receive(:call).and_return(3) + + action + + json = JSON.parse(response.body, symbolize_names: true) + expect(response.status).to eq(400) + expect(json).to eq({ + success: false, + errors: [{ field: 'front', message: 'Please fill in this field.' }], + remaining_attempts: 3, + }) + end + + it 'returns an error when throttled' do + allow(Throttler::IsThrottledElseIncrement).to receive(:call).once.and_return(true) + allow(Throttler::RemainingCount).to receive(:call).and_return(0) + + action + + json = JSON.parse(response.body, symbolize_names: true) + expect(response.status).to eq(429) + expect(json).to eq({ + success: false, + errors: [{ + field: 'limit', + message: I18n.t('errors.doc_auth.acuant_throttle'), + }], + remaining_attempts: 0, + }) + end + end + context 'when image upload succeeds' do it 'returns a successful response and modifies the session' do action json = JSON.parse(response.body, symbolize_names: true) + expect(response.status).to eq(200) expect(json[:success]).to eq(true) - expect(document_capture_session.reload.load_result.success?).to eq(true) end end @@ -101,7 +139,9 @@ action json = JSON.parse(response.body, symbolize_names: true) + expect(response.status).to eq(400) expect(json[:success]).to eq(false) + expect(json[:remaining_attempts]).to be_a_kind_of(Numeric) expect(json[:errors]).to eq [ { field: 'front', message: 'Too blurry' }, { field: 'front', message: 'Wrong document' }, @@ -116,6 +156,7 @@ action json = JSON.parse(response.body, symbolize_names: true) + expect(json[:remaining_attempts]).to be_a_kind_of(Numeric) expect(json[:errors]).to eq [ { field: 'results', diff --git a/spec/forms/idv/api_image_upload_form_spec.rb b/spec/forms/idv/api_image_upload_form_spec.rb index 7e3b4dffe44..039e29d2723 100644 --- a/spec/forms/idv/api_image_upload_form_spec.rb +++ b/spec/forms/idv/api_image_upload_form_spec.rb @@ -76,5 +76,24 @@ expect(form.errors[:document_capture_session]).to eq(['Please fill in this field.']) end end + + context 'when throttled from submission' do + before do + allow(Throttler::IsThrottledElseIncrement).to receive(:call).once.and_return(true) + form.submit + end + + it 'is not valid' do + expect(form.valid?).to eq(false) + expect(form.errors[:limit]).to eq([I18n.t('errors.doc_auth.acuant_throttle')]) + end + end + end + + describe '#submit' do + it 'includes remaining_attempts' do + response = form.submit + expect(response.extra[:remaining_attempts]).to be_a_kind_of(Numeric) + end end end diff --git a/spec/presenters/image_upload_response_presenter_spec.rb b/spec/presenters/image_upload_response_presenter_spec.rb new file mode 100644 index 00000000000..fdeb88633e8 --- /dev/null +++ b/spec/presenters/image_upload_response_presenter_spec.rb @@ -0,0 +1,72 @@ +require 'rails_helper' + +describe ImageUploadResponsePresenter do + let(:form) { Idv::ApiImageUploadForm.new({}, liveness_checking_enabled: false) } + let(:form_response) { FormResponse.new(success: true, errors: {}, extra: {}) } + let(:presenter) { described_class.new(form: form, form_response: form_response) } + + before do + allow(Throttler::RemainingCount).to receive(:call).and_return(3) + allow(DocumentCaptureSession).to receive(:find_by).and_return( + DocumentCaptureSession.create!(requested_at: Time.zone.now), + ) + end + + describe '#success' do + context 'failure' do + let(:form_response) { FormResponse.new(success: false, errors: {}, extra: {}) } + + it 'returns false' do + expect(presenter.success).to eq false + end + end + + context 'success' do + it 'returns true' do + expect(presenter.success).to eq true + end + end + end + + describe '#errors' do + context 'failure' do + let(:form_response) do + FormResponse.new( + success: false, + errors: { + front: t('doc_auth.errors.not_a_file'), + }, + extra: {}, + ) + end + + it 'returns formatted errors' do + expect(presenter.errors).to eq [{ field: :front, message: t('doc_auth.errors.not_a_file') }] + end + end + + context 'success' do + it 'returns empty array' do + expect(presenter.errors).to eq [] + end + end + end + + describe '#remaining_attempts' do + it 'returns remaining attempts' do + expect(presenter.remaining_attempts).to eq 3 + end + end + + describe '#as_json' do + it 'returns hash of properties' do + expected = { + success: true, + errors: [], + remaining_attempts: 3, + } + + expect(presenter.as_json).to eq expected + end + end +end