Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion app/controllers/concerns/idv_session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ def idv_session
end

def idv_attempter_throttled?
Throttler::IsThrottled.call(effective_user.id, :idv_resolution)
Throttle.for(
user: effective_user,
throttle_type: :idv_resolution,
).throttled?
end

def redirect_unless_effective_user
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/idv/capture_doc_status_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ def document_capture_session
end

def throttled?
Throttler::IsThrottled.call(document_capture_session.user_id, :idv_acuant)
Throttle.for(
user: document_capture_session.user,
throttle_type: :idv_acuant,
).throttled?
end
end
end
9 changes: 6 additions & 3 deletions app/controllers/idv/gpo_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,18 @@ def form_response(result, success)
end

def idv_throttle_params
[idv_session.current_user.id, :idv_resolution]
{
user: idv_session.current_user,
throttle_type: :idv_resolution,
}
end

def idv_attempter_increment
Throttler::Increment.call(*idv_throttle_params)
Throttle.for(**idv_throttle_params).increment
end

def idv_attempter_throttled?
Throttler::IsThrottled.call(*idv_throttle_params)
Throttle.for(**idv_throttle_params).throttled?
end

def throttle_failure
Expand Down
13 changes: 5 additions & 8 deletions app/controllers/idv/session_errors_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module Idv
class SessionErrorsController < ApplicationController
include IdvSession
include EffectiveUser

before_action :confirm_two_factor_authenticated_or_user_id_in_session
before_action :confirm_idv_session_step_needed
Expand All @@ -12,7 +13,10 @@ def warning
private

def remaining_step_attempts
Throttler::RemainingCount.call(user_id, :idv_resolution)
Throttle.for(
user: effective_user,
throttle_type: :idv_resolution,
).remaining_count
end

def confirm_two_factor_authenticated_or_user_id_in_session
Expand All @@ -25,12 +29,5 @@ def confirm_idv_session_step_needed
return unless user_fully_authenticated?
redirect_to idv_phone_url if idv_session.profile_confirmation == true
end

def user_id
doc_capture_user_id = session[:doc_capture_user_id]
return doc_capture_user_id if doc_capture_user_id.present?

current_user.id
end
end
end
16 changes: 9 additions & 7 deletions app/controllers/users/verify_account_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def index
@verify_account_form = VerifyAccountForm.new(user: current_user)
@code = session[:last_gpo_confirmation_code] if FeatureManagement.reveal_gpo_code?

if Throttler::IsThrottled.call(current_user.id, :verify_gpo_key)
if throttle.throttled?
render_throttled
else
render :index
Expand All @@ -22,12 +22,7 @@ def index
def create
@verify_account_form = build_verify_account_form

throttled = Throttler::IsThrottledElseIncrement.call(
current_user.id,
:verify_gpo_key,
)

if throttled
if throttle.throttled_else_increment?
render_throttled
else
result = @verify_account_form.submit
Expand All @@ -52,6 +47,13 @@ def create

private

def throttle
@throttle ||= Throttle.for(
user: current_user,
throttle_type: :verify_gpo_key,
)
end

def render_throttled
analytics.track_event(
Analytics::THROTTLER_RATE_LIMIT_TRIGGERED,
Expand Down
16 changes: 9 additions & 7 deletions app/controllers/users/verify_personal_key_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,15 @@ def new
personal_key: '',
)

if Throttler::IsThrottled.call(current_user.id, :verify_personal_key)
if throttle.throttled?
render_throttled
else
render :new
end
end

def create
throttled = Throttler::IsThrottledElseIncrement.call(
current_user.id,
:verify_personal_key,
)

if throttled
if throttle.throttled_else_increment?
render_throttled
else
result = personal_key_form.submit
Expand All @@ -42,6 +37,13 @@ def create

private

def throttle
@throttle ||= Throttle.for(
user: current_user,
throttle_type: :verify_personal_key,
)
end

def render_throttled
analytics.track_event(
Analytics::THROTTLER_RATE_LIMIT_TRIGGERED,
Expand Down
12 changes: 8 additions & 4 deletions app/forms/idv/api_document_verification_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def submit

def remaining_attempts
return unless document_capture_session
Throttler::RemainingCount.call(document_capture_session.user_id, :idv_acuant)
throttle.remaining_count
end

def liveness_checking_enabled?
Expand Down Expand Up @@ -95,9 +95,13 @@ def throttle_if_rate_limited

def throttled_else_increment
return unless document_capture_session
@throttled = Throttler::IsThrottledElseIncrement.call(
document_capture_session.user_id,
:idv_acuant,
@throttled = throttle.throttled_else_increment?
end

def throttle
@throttle ||= Throttle.for(
user: document_capture_session.user,
throttle_type: :idv_acuant,
)
end

Expand Down
5 changes: 4 additions & 1 deletion app/forms/idv/api_document_verification_status_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ def submit

def remaining_attempts
return unless @document_capture_session
Throttler::RemainingCount.call(@document_capture_session.user_id, :idv_acuant)
Throttle.for(
user: @document_capture_session.user,
throttle_type: :idv_acuant,
).remaining_count
end

def timeout_error
Expand Down
14 changes: 9 additions & 5 deletions app/forms/idv/api_image_upload_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,7 @@ def submit

def throttled_else_increment
return unless document_capture_session
@throttled = Throttler::IsThrottledElseIncrement.call(
document_capture_session.user_id,
:idv_acuant,
)
@throttled = throttle.throttled_else_increment?
end

def validate_form
Expand Down Expand Up @@ -104,7 +101,7 @@ def extra_attributes

def remaining_attempts
return nil unless document_capture_session
Throttler::RemainingCount.call(document_capture_session.user_id, :idv_acuant)
throttle.remaining_count
end

def determine_response(form_response:, client_response:, doc_pii_response:)
Expand Down Expand Up @@ -247,5 +244,12 @@ def user_id
def user_uuid
document_capture_session&.user&.uuid
end

def throttle
@throttle ||= Throttle.for(
user: document_capture_session.user,
throttle_type: :idv_acuant,
)
end
end
end
6 changes: 4 additions & 2 deletions app/forms/register_user_email_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ def process_errors(request_id)
end

def send_sign_up_unconfirmed_email(request_id)
@throttled = Throttler::IsThrottledElseIncrement.call(existing_user.id, :reg_unconfirmed_email)
throttler = Throttle.for(user: existing_user, throttle_type: :reg_unconfirmed_email)
@throttled = throttler.throttled_else_increment?

if @throttled
@analytics.track_event(
Expand All @@ -146,7 +147,8 @@ def send_sign_up_unconfirmed_email(request_id)
end

def send_sign_up_confirmed_email
@throttled = Throttler::IsThrottledElseIncrement.call(existing_user.id, :reg_confirmed_email)
throttler = Throttle.for(user: existing_user, throttle_type: :reg_confirmed_email)
@throttled = throttler.throttled_else_increment?

if @throttled
@analytics.track_event(
Expand Down
5 changes: 1 addition & 4 deletions app/jobs/document_proofing_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ def perform(
pii: proofer_result.pii_from_doc,
)

remaining_attempts = Throttler::RemainingCount.call(
user.id,
:idv_acuant,
)
remaining_attempts = Throttle.for(user: user, throttle_type: :idv_acuant).remaining_count

analytics.track_event(
Analytics::IDV_DOC_AUTH_SUBMITTED_IMAGE_UPLOAD_VENDOR,
Expand Down
48 changes: 47 additions & 1 deletion app/models/throttle.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class Throttle < ApplicationRecord
belongs_to :user
validates :user_id, presence: true
validates :user_id, presence: true, unless: :target?
validates :target, presence: true, unless: :user_id?

enum throttle_type: {
idv_acuant: 1,
Expand Down Expand Up @@ -48,10 +49,49 @@ class Throttle < ApplicationRecord
},
}.freeze

# Either target or user must be supplied
# @param [Symbol] throttle_type
# @param [String] target
# @param [User] user
# @return [Throttle]
def self.for(throttle_type:, user: nil, target: nil)
throttle = if user
find_or_create_by(user: user, throttle_type: throttle_type)
elsif target
find_or_create_by(target: target, throttle_type: throttle_type)
else
raise 'Throttle must have a user or a target, but neither were provided'
end

throttle.reset_if_expired_and_maxed
throttle
end

# @return [Integer]
def increment
return attempts if maxed?
update(attempts: attempts + 1, attempted_at: Time.zone.now)
attempts
end

def throttled?
!expired? && maxed?
end

def throttled_else_increment?
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.

Open to new name suggestions here! It's not a normal predicate method because it has a side effect. Maybe just drop the ? at the end? 😬

if throttled?
true
else
update(attempts: attempts + 1, attempted_at: Time.zone.now)
false
end
end

def reset
update(attempts: 0)
self
end

def remaining_count
return 0 if throttled?
max_attempts, _attempt_window_in_minutes = Throttle.config_values(throttle_type)
Expand All @@ -73,4 +113,10 @@ def self.config_values(throttle_type)
config = THROTTLE_CONFIG.with_indifferent_access[throttle_type]
[config[:max_attempts], config[:attempt_window]]
end

# @api private
def reset_if_expired_and_maxed
return unless expired? && maxed?
update(attempts: 0, throttled_count: throttled_count.to_i + 1)
end
end
5 changes: 4 additions & 1 deletion app/services/idv/actions/verify_document_status_action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ def async_state

def remaining_attempts
return nil unless verify_document_capture_session
Throttler::RemainingCount.call(verify_document_capture_session.user_id, :idv_acuant)
Throttle.for(
user: verify_document_capture_session.user,
throttle_type: :idv_acuant,
).remaining_count
end

def timed_out
Expand Down
14 changes: 10 additions & 4 deletions app/services/idv/steps/doc_auth_base_step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@ def initialize(flow)
private

def idv_throttle_params
[current_user.id, :idv_resolution]
{
user: current_user,
throttle_type: :idv_resolution,
}
end

def attempter_increment
Throttler::Increment.call(*idv_throttle_params)
Throttle.for(**idv_throttle_params).increment
end

def attempter_throttled?
Throttler::IsThrottled.call(*idv_throttle_params)
Throttle.for(**idv_throttle_params).throttled?
end

def idv_failure(result)
Expand Down Expand Up @@ -87,7 +90,10 @@ def throttled_url
end

def throttled_else_increment
Throttler::IsThrottledElseIncrement.call(user_id, :idv_acuant)
Throttle.for(
target: user_id,
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.

Should this have been... ?

Suggested change
target: user_id,
user: user_id,

Copy link
Copy Markdown
Contributor Author

@zachmargolis zachmargolis Aug 30, 2021

Choose a reason for hiding this comment

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

🤦 I think it should have been:

Suggested change
target: user_id,
user: current_user,

but yes

throttle_type: :idv_acuant,
).throttled_else_increment?
end

def user_id
Expand Down
5 changes: 4 additions & 1 deletion app/services/idv/steps/send_link_step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ def link(session_uuid)
end

def throttled_else_increment
Throttler::IsThrottledElseIncrement.call(user_id, :idv_send_link)
Throttle.for(
user: current_user,
throttle_type: :idv_send_link,
).throttled_else_increment?
end
end
end
Expand Down
3 changes: 1 addition & 2 deletions app/services/request_password_reset.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ def perform
private

def send_reset_password_instructions
throttled = Throttler::IsThrottledElseIncrement.call(user.id, :reset_password_email)
if throttled
if Throttle.for(user: user, throttle_type: :reset_password_email).throttled_else_increment?
analytics.track_event(
Analytics::THROTTLER_RATE_LIMIT_TRIGGERED,
throttle_type: :reset_password_email,
Expand Down
Loading