Skip to content

Commit

Permalink
Merge pull request Houdini#4 from selma-finance/no-otp-resend-in-api
Browse files Browse the repository at this point in the history
Don't resend otp code to user if we're in one of the configured scopes to skip it
  • Loading branch information
jarkko authored Oct 7, 2020
2 parents 431712c + db297e9 commit d205779
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 9 deletions.
3 changes: 3 additions & 0 deletions lib/two_factor_authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ module Devise
mattr_accessor :otp_secret_encryption_key
@@otp_secret_encryption_key = ''

mattr_accessor :skip_send_new_otp_in_after_set_user_for
@@skip_send_new_otp_in_after_set_user_for = []

mattr_accessor :second_factor_resource_id
@@second_factor_resource_id = 'id'

Expand Down
19 changes: 11 additions & 8 deletions lib/two_factor_authentication/hooks/two_factor_authenticatable.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
Warden::Manager.after_authentication do |user, auth, options|
if auth.env["action_dispatch.cookies"]
expected_cookie_value = "#{user.class}-#{user.public_send(Devise.second_factor_resource_id)}"
actual_cookie_value = auth.env["action_dispatch.cookies"].signed[TwoFactorAuthentication::REMEMBER_TFA_COOKIE_NAME]
bypass_by_cookie = actual_cookie_value == expected_cookie_value
end
scopes_to_skip = [Rails.application.config.devise.skip_send_new_otp_in_after_set_user_for].compact.flatten
unless options[:scope].in?(scopes_to_skip)
if auth.env["action_dispatch.cookies"]
expected_cookie_value = "#{user.class}-#{user.public_send(Devise.second_factor_resource_id)}"
actual_cookie_value = auth.env["action_dispatch.cookies"].signed[TwoFactorAuthentication::REMEMBER_TFA_COOKIE_NAME]
bypass_by_cookie = actual_cookie_value == expected_cookie_value
end

if user.respond_to?(:need_two_factor_authentication?) && !bypass_by_cookie
if auth.session(options[:scope])[TwoFactorAuthentication::NEED_AUTHENTICATION] = user.need_two_factor_authentication?(auth.request)
user.send_new_otp if user.send_new_otp_after_login?
if user.respond_to?(:need_two_factor_authentication?) && !bypass_by_cookie
if auth.session(options[:scope])[TwoFactorAuthentication::NEED_AUTHENTICATION] = user.need_two_factor_authentication?(auth.request)
user.send_new_otp if user.send_new_otp_after_login?
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def authenticate_totp(code, options = {})
raise "authenticate_totp called with no otp_secret_key set" if totp_secret.nil?
totp = ROTP::TOTP.new(totp_secret, digits: digits)
new_timestamp = totp.verify(
without_spaces(code),
without_spaces(code),
drift_ahead: drift, drift_behind: drift, after: totp_timestamp
)
return false unless new_timestamp
Expand Down
15 changes: 15 additions & 0 deletions spec/features/two_factor_authenticatable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,21 @@
expect(page).to have_content("Enter the code that was sent to you")
end

scenario 'Sends OTP code by SMS' do
login_as user
SMSProvider.messages.clear()
visit dashboard_path
expect(SMSProvider.messages).not_to be_empty
end

scenario "Doesn't sends OTP code by SMS upon every request if so configured" do
login_as user
SMSProvider.messages.clear()
allow(Rails.application.config.devise).to receive(:skip_send_new_otp_in_after_set_user_for).and_return([:user])
visit dashboard_path
expect(SMSProvider.messages).to be_empty
end

scenario 'TFA should be different for different users' do
sms_sign_in

Expand Down
1 change: 1 addition & 0 deletions spec/rails_app/config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -255,4 +255,5 @@
# config.omniauth_path_prefix = '/my_engine/users/auth'

config.otp_secret_encryption_key = '0a8283fba984da1de24e4df1e93046cb53c5787944ef037b2dbf3e61d20fe11f25e25a855cec605fdf65b162329890d7230afdf64f681b4c32020281054e73ec'
config.skip_send_new_otp_in_after_set_user_for = []
end

0 comments on commit d205779

Please sign in to comment.