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
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def sign_out(*args)
end

def context
user_session[:context] || UserSessionContext::DEFAULT_CONTEXT
user_session[:context] || UserSessionContext::AUTHENTICATION_CONTEXT
end

def current_sp
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/remember_device_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def save_remember_device_preference
end

def check_remember_device_preference
return unless UserSessionContext.authentication_context?(context)
return unless UserSessionContext.authentication_or_reauthentication_context?(context)
return if remember_device_cookie.nil?
return unless remember_device_cookie.valid_for_user?(
user: current_user,
Expand Down
12 changes: 6 additions & 6 deletions app/controllers/concerns/two_factor_authenticatable_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def handle_second_factor_locked_user(type:, context: nil)
PushNotification::HttpPush.deliver(event)

if context && type
if UserSessionContext.authentication_context?(context)
if UserSessionContext.authentication_or_reauthentication_context?(context)
irs_attempts_api_tracker.mfa_login_rate_limited(mfa_device_type: type)
elsif UserSessionContext.confirmation_context?(context)
irs_attempts_api_tracker.mfa_enroll_rate_limited(mfa_device_type: type)
Expand All @@ -37,7 +37,7 @@ def handle_too_many_otp_sends(phone: nil, context: nil)
analytics.multi_factor_auth_max_sends

if context && phone
if UserSessionContext.authentication_context?(context)
if UserSessionContext.authentication_or_reauthentication_context?(context)
irs_attempts_api_tracker.mfa_login_phone_otp_sent_rate_limited(
phone_number: phone,
)
Expand Down Expand Up @@ -69,7 +69,7 @@ def current_password_required?
end

def check_already_authenticated
return unless UserSessionContext.initial_authentication_context?(context)
return unless UserSessionContext.authentication_context?(context)
return unless user_fully_authenticated?
return if remember_device_expired_for_sp?
return if service_provider_mfa_policy.user_needs_sp_auth_method_verification?
Expand Down Expand Up @@ -112,7 +112,7 @@ def handle_remember_device
end

def handle_valid_otp_for_context
if UserSessionContext.authentication_context?(context)
if UserSessionContext.authentication_or_reauthentication_context?(context)
handle_valid_otp_for_authentication_context
elsif UserSessionContext.confirmation_context?(context)
handle_valid_otp_for_confirmation_context
Expand Down Expand Up @@ -313,15 +313,15 @@ def generic_data
end

def display_phone_to_deliver_to
if UserSessionContext.authentication_context?(context)
if UserSessionContext.authentication_or_reauthentication_context?(context)
phone_configuration.masked_phone
else
user_session[:unconfirmed_phone]
end
end

def voice_otp_delivery_unsupported?
if UserSessionContext.authentication_context?(context)
if UserSessionContext.authentication_or_reauthentication_context?(context)
PhoneNumberCapabilities.new(phone_configuration&.phone, phone_confirmed: true).supports_voice?
else
phone = user_session[:unconfirmed_phone]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def phone_enabled?
def confirm_voice_capability
return if two_factor_authentication_method == 'sms'

phone_is_confirmed = UserSessionContext.authentication_context?(context)
phone_is_confirmed = UserSessionContext.authentication_or_reauthentication_context?(context)

capabilities = PhoneNumberCapabilities.new(phone, phone_confirmed: phone_is_confirmed)

Expand Down Expand Up @@ -98,7 +98,7 @@ def post_analytics(result)
reauthentication: true,
success: properties[:success],
)
elsif UserSessionContext.authentication_context?(context)
elsif UserSessionContext.authentication_or_reauthentication_context?(context)
irs_attempts_api_tracker.mfa_login_phone_otp_submitted(
reauthentication: false,
success: properties[:success],
Expand Down
12 changes: 7 additions & 5 deletions app/controllers/users/two_factor_authentication_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def phone_configuration
def validate_otp_delivery_preference_and_send_code
result = otp_delivery_selection_form.submit(otp_delivery_preference: delivery_preference)
analytics.otp_delivery_selection(**result.to_h)
phone_is_confirmed = UserSessionContext.authentication_context?(context)
phone_is_confirmed = UserSessionContext.authentication_or_reauthentication_context?(context)
phone_capabilities = PhoneNumberCapabilities.new(
parsed_phone,
phone_confirmed: phone_is_confirmed,
Expand Down Expand Up @@ -230,7 +230,7 @@ def track_events(otp_delivery_preference:)
phone_number: parsed_phone.e164,
otp_delivery_method: otp_delivery_preference,
)
elsif UserSessionContext.authentication_context?(context)
elsif UserSessionContext.authentication_or_reauthentication_context?(context)
irs_attempts_api_tracker.mfa_login_phone_otp_sent(
success: @telephony_result.success?,
reauthentication: false,
Expand Down Expand Up @@ -280,7 +280,7 @@ def send_user_otp(method)
country_code: parsed_phone.country,
}

if UserSessionContext.authentication_context?(context)
if UserSessionContext.authentication_or_reauthentication_context?(context)
Telephony.send_authentication_otp(**params)
else
Telephony.send_confirmation_otp(**params)
Expand All @@ -304,7 +304,9 @@ def delivery_params
end

def phone_to_deliver_to
return phone_configuration&.phone if UserSessionContext.authentication_context?(context)
if UserSessionContext.authentication_or_reauthentication_context?(context)
return phone_configuration&.phone
end

user_session[:unconfirmed_phone]
end
Expand All @@ -313,7 +315,7 @@ def otp_rate_limiter
@otp_rate_limiter ||= OtpRateLimiter.new(
phone: phone_to_deliver_to,
user: current_user,
phone_confirmed: UserSessionContext.authentication_context?(context),
phone_confirmed: UserSessionContext.authentication_or_reauthentication_context?(context),
)
end

Expand Down
2 changes: 1 addition & 1 deletion app/forms/otp_delivery_selection_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@ def parsed_phone
end

def confirmed_phone?
UserSessionContext.authentication_context?(context)
UserSessionContext.authentication_or_reauthentication_context?(context)
end
end
10 changes: 5 additions & 5 deletions app/services/user_session_context.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
class UserSessionContext
DEFAULT_CONTEXT = 'authentication'.freeze
AUTHENTICATION_CONTEXT = 'authentication'.freeze
REAUTHENTICATION_CONTEXT = 'reauthentication'.freeze
CONFIRMATION_CONTEXT = 'confirmation'.freeze

def self.initial_authentication_context?(context)
context == DEFAULT_CONTEXT
def self.authentication_context?(context)
context == AUTHENTICATION_CONTEXT
end

def self.reauthentication_context?(context)
context == REAUTHENTICATION_CONTEXT
end

def self.authentication_context?(context)
initial_authentication_context?(context) || reauthentication_context?(context)
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.

I forgot what a footgun this method was until you picked up this story. Thanks for fixing this!

def self.authentication_or_reauthentication_context?(context)
authentication_context?(context) || reauthentication_context?(context)
end

def self.confirmation_context?(context)
Expand Down
4 changes: 2 additions & 2 deletions spec/presenters/two_factor_login_options_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
let(:view) { ActionController::Base.new.view_context }
let(:phishing_resistant_required) { false }
let(:piv_cac_required) { false }
let(:user_session_context) { UserSessionContext::DEFAULT_CONTEXT }
let(:user_session_context) { UserSessionContext::AUTHENTICATION_CONTEXT }

subject(:presenter) do
TwoFactorLoginOptionsPresenter.new(
Expand Down Expand Up @@ -96,7 +96,7 @@
subject(:cancel_link) { presenter.cancel_link }

context 'default user session context' do
let(:user_session_context) { UserSessionContext::DEFAULT_CONTEXT }
let(:user_session_context) { UserSessionContext::AUTHENTICATION_CONTEXT }

it { should eq sign_out_path }
end
Expand Down
24 changes: 14 additions & 10 deletions spec/services/user_session_context_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@
describe UserSessionContext do
let(:confirmation) { { context: 'confirmation' } }

describe '.initial_authentication_context?' do
describe '.authentication_context?' do
it 'returns true when context is default context' do
expect(
UserSessionContext.initial_authentication_context?(UserSessionContext::DEFAULT_CONTEXT),
UserSessionContext.authentication_context?(UserSessionContext::AUTHENTICATION_CONTEXT),
).to eq true
end

it 'returns false when context is not default context' do
expect(
UserSessionContext.initial_authentication_context?(
UserSessionContext.authentication_context?(
UserSessionContext::CONFIRMATION_CONTEXT,
),
).to eq false

expect(
UserSessionContext.initial_authentication_context?(
UserSessionContext.authentication_context?(
UserSessionContext::REAUTHENTICATION_CONTEXT,
),
).to eq false
Expand All @@ -34,25 +34,29 @@

it 'returns false when context is default context' do
expect(
UserSessionContext.reauthentication_context?(UserSessionContext::DEFAULT_CONTEXT),
UserSessionContext.reauthentication_context?(UserSessionContext::AUTHENTICATION_CONTEXT),
).to eq false
end
end

describe '.authentication_context?' do
describe '.authentication_or_reauthentication_context?' do
it 'returns true when context is default or reauth context' do
expect(
UserSessionContext.authentication_context?(UserSessionContext::DEFAULT_CONTEXT),
UserSessionContext.authentication_or_reauthentication_context?(
UserSessionContext::AUTHENTICATION_CONTEXT,
),
).to eq true

expect(
UserSessionContext.authentication_context?(UserSessionContext::REAUTHENTICATION_CONTEXT),
UserSessionContext.authentication_or_reauthentication_context?(
UserSessionContext::REAUTHENTICATION_CONTEXT,
),
).to eq true
end

it 'returns false when context is confirmation context' do
expect(
UserSessionContext.initial_authentication_context?(
UserSessionContext.authentication_context?(
UserSessionContext::CONFIRMATION_CONTEXT,
),
).to eq false
Expand All @@ -68,7 +72,7 @@

it 'returns false when context is default or reauth context' do
expect(
UserSessionContext.confirmation_context?(UserSessionContext::DEFAULT_CONTEXT),
UserSessionContext.confirmation_context?(UserSessionContext::AUTHENTICATION_CONTEXT),
).to eq false

expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
@presenter = TwoFactorLoginOptionsPresenter.new(
user: user,
view: view,
user_session_context: UserSessionContext::DEFAULT_CONTEXT,
user_session_context: UserSessionContext::AUTHENTICATION_CONTEXT,
service_provider: nil,
phishing_resistant_required: false,
piv_cac_required: false,
Expand Down