diff --git a/Rakefile b/Rakefile
index 7723dfbb..9a575025 100644
--- a/Rakefile
+++ b/Rakefile
@@ -1,3 +1,9 @@
+begin
+ require 'bundler/setup'
+rescue LoadError
+ puts 'You must `gem install bundler` and `bundle install` to run rake tasks'
+end
+
require "bundler/gem_tasks"
APP_RAKEFILE = File.expand_path("../spec/rails_app/Rakefile", __FILE__)
diff --git a/app/views/devise/two_factor_authentication/show.html.erb b/app/views/devise/two_factor_authentication/show.html.erb
index 9a3ae060..77f3707e 100644
--- a/app/views/devise/two_factor_authentication/show.html.erb
+++ b/app/views/devise/two_factor_authentication/show.html.erb
@@ -12,8 +12,8 @@
<% end %>
<% if resource.direct_otp %>
- <%= link_to "Resend Code", send("resend_code_#{resource_name}_two_factor_authentication_path"), action: :get %>
+ <%= link_to "Resend Code", send("resend_code_#{resource_name}_two_factor_authentication_path"), method: :post %>
<% else %>
-<%= link_to "Send me a code instead", send("resend_code_#{resource_name}_two_factor_authentication_path"), action: :get %>
+<%= link_to "Send me a code instead", send("resend_code_#{resource_name}_two_factor_authentication_path"), method: :post %>
<% end %>
-<%= link_to "Sign out", send("destroy_#{resource_name}_session_path"), :method => :delete %>
+<%= link_to "Sign out", send("destroy_#{resource_name}_session_path"), method: :delete %>
diff --git a/lib/two_factor_authentication.rb b/lib/two_factor_authentication.rb
index 7b1bbbc1..247b541f 100644
--- a/lib/two_factor_authentication.rb
+++ b/lib/two_factor_authentication.rb
@@ -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'
diff --git a/lib/two_factor_authentication/controllers/helpers.rb b/lib/two_factor_authentication/controllers/helpers.rb
index 64e8377c..501f4d9a 100644
--- a/lib/two_factor_authentication/controllers/helpers.rb
+++ b/lib/two_factor_authentication/controllers/helpers.rb
@@ -20,14 +20,15 @@ def handle_two_factor_authentication
end
def handle_failed_second_factor(scope)
- if request.format.present?
- if request.format.html?
- session["#{scope}_return_to"] = request.original_fullpath if request.get?
- redirect_to two_factor_authentication_path_for(scope)
- elsif request.format.json?
- session["#{scope}_return_to"] = root_path(format: :html)
- render json: { redirect_to: two_factor_authentication_path_for(scope) }, status: :unauthorized
- end
+ if request.format&.html?
+ session["#{scope}_return_to"] = request.original_fullpath if request.get?
+ redirect_to two_factor_authentication_path_for(scope)
+ elsif request.format&.json?
+ session["#{scope}_return_to"] = root_path(format: :html)
+ render json: {
+ redirect_to: two_factor_authentication_path_for(scope),
+ authentication_type: send("current_#{scope}")&.direct_otp ? :otp : :totp
+ }, status: :unauthorized
else
head :unauthorized
end
diff --git a/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb b/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb
index 3ff03415..28a2b0f7 100644
--- a/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb
+++ b/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb
@@ -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
diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb
index 6d73a0fb..0e85c490 100644
--- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb
+++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb
@@ -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
@@ -101,7 +101,7 @@ def generate_totp_secret
def create_direct_otp(options = {})
# Create a new random OTP and store it in the database
digits = options[:length] || self.class.direct_otp_length || 6
- update_attributes(
+ update(
direct_otp: random_base10(digits),
direct_otp_sent_at: Time.now.utc
)
@@ -122,7 +122,7 @@ def direct_otp_expired?
end
def clear_direct_otp
- update_attributes(direct_otp: nil, direct_otp_sent_at: nil)
+ update(direct_otp: nil, direct_otp_sent_at: nil)
end
end
diff --git a/lib/two_factor_authentication/routes.rb b/lib/two_factor_authentication/routes.rb
index 543059a2..1c1a8b26 100644
--- a/lib/two_factor_authentication/routes.rb
+++ b/lib/two_factor_authentication/routes.rb
@@ -2,10 +2,16 @@ module ActionDispatch::Routing
class Mapper
protected
- def devise_two_factor_authentication(mapping, controllers)
- resource :two_factor_authentication, :only => [:show, :update, :resend_code], :path => mapping.path_names[:two_factor_authentication], :controller => controllers[:two_factor_authentication] do
- collection { get "resend_code" }
- end
- end
+ def devise_two_factor_authentication(mapping, controllers)
+ path = mapping.path_names[:two_factor_authentication]
+ controller = controllers[:two_factor_authentication]
+
+ resource :two_factor_authentication,
+ only: [:show, :update],
+ path: path,
+ controller: controller
+
+ post "#{path}/resend_code", to: "#{controller}#resend_code"
+ end
end
end
diff --git a/spec/features/two_factor_authenticatable_spec.rb b/spec/features/two_factor_authenticatable_spec.rb
index d433d636..e31a9f9b 100644
--- a/spec/features/two_factor_authenticatable_spec.rb
+++ b/spec/features/two_factor_authenticatable_spec.rb
@@ -76,6 +76,22 @@
expect(page).to have_content("Param B is param b")
end
+ scenario "is redirected to TFA when path requires authentication and works with resent code" do
+ visit dashboard_path + "?A=param%20a&B=param%20b"
+
+ expect(page).to_not have_content("Your Personal Dashboard")
+ SMSProvider.messages.clear()
+
+ click_on "Resend Code"
+ fill_in "code", with: SMSProvider.last_message.body
+ click_button "Submit"
+
+ expect(page).to have_content("Your Personal Dashboard")
+ expect(page).to have_content("You are signed in as Marissa")
+ expect(page).to have_content("Param A is param a")
+ expect(page).to have_content("Param B is param b")
+ end
+
scenario "is locked out after max failed attempts" do
visit user_two_factor_authentication_path
@@ -136,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
@@ -160,6 +191,14 @@ def sms_sign_in
click_button 'Submit'
end
+ def sms_sign_in_with_resent_code
+ visit user_two_factor_authentication_path
+ SMSProvider.messages.clear()
+ click_on 'Resend Code'
+ fill_in 'code', with: SMSProvider.last_message.body
+ click_button 'Submit'
+ end
+
scenario 'TFA should be unique for specific user' do
sms_sign_in
@@ -175,6 +214,21 @@ def sms_sign_in
expect(page).to have_content("Enter the code that was sent to you")
end
+ scenario 'TFA should be unique for specific user with resent code' do
+ sms_sign_in_with_resent_code
+
+ tfa_cookie1 = get_tfa_cookie()
+
+ logout
+ reset_session!
+
+ user2 = create_user()
+ set_tfa_cookie(tfa_cookie1)
+ login_as(user2)
+ visit dashboard_path
+ expect(page).to have_content("Enter the code that was sent to you")
+ end
+
scenario 'Delete cookie when user logs out if enabled' do
user.class.delete_cookie_on_logout = true
diff --git a/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb b/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb
index 6fb4f505..6e58fd1d 100644
--- a/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb
+++ b/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb
@@ -138,7 +138,7 @@ def instance.send_two_factor_authentication_code(code)
it "returns uri with user's email" do
expect(instance.provisioning_uri).
- to match(%r{otpauth://totp/houdini@example.com\?secret=\w{32}})
+ to match(%r{otpauth://totp/houdini%40example.com\?secret=\w{32}})
end
it 'returns uri with issuer option' do
diff --git a/spec/rails_app/app/assets/config/manifest.js b/spec/rails_app/app/assets/config/manifest.js
new file mode 100644
index 00000000..a3d7d420
--- /dev/null
+++ b/spec/rails_app/app/assets/config/manifest.js
@@ -0,0 +1,2 @@
+//= link application.css
+//= link application.js
diff --git a/spec/rails_app/app/controllers/home_controller.rb b/spec/rails_app/app/controllers/home_controller.rb
index f55da102..7007d886 100644
--- a/spec/rails_app/app/controllers/home_controller.rb
+++ b/spec/rails_app/app/controllers/home_controller.rb
@@ -5,6 +5,15 @@ def index
end
def dashboard
+ respond_to do |format|
+ format.html
+ format.json do
+ render json: {success: true}
+ end
+ format.xml do
+ render xml: ""
+ end
+ end
end
end
diff --git a/spec/rails_app/app/models/guest_user.rb b/spec/rails_app/app/models/guest_user.rb
index 8003624c..1222279a 100644
--- a/spec/rails_app/app/models/guest_user.rb
+++ b/spec/rails_app/app/models/guest_user.rb
@@ -7,7 +7,7 @@ class GuestUser
attr_accessor :direct_otp, :direct_otp_sent_at, :otp_secret_key, :email,
:second_factor_attempts_count, :totp_timestamp
- def update_attributes(attrs)
+ def update(attrs)
attrs.each do |key, value|
send(key.to_s + '=', value)
end
diff --git a/spec/rails_app/config/initializers/devise.rb b/spec/rails_app/config/initializers/devise.rb
index 9083d7f4..6a714f08 100644
--- a/spec/rails_app/config/initializers/devise.rb
+++ b/spec/rails_app/config/initializers/devise.rb
@@ -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
diff --git a/spec/requests/api_request_spec.rb b/spec/requests/api_request_spec.rb
new file mode 100644
index 00000000..c7b48ad8
--- /dev/null
+++ b/spec/requests/api_request_spec.rb
@@ -0,0 +1,65 @@
+require 'spec_helper'
+
+describe "API request", type: :request do
+ context "when logged in" do
+ let(:user) { create_user("encrypted", email: "foo@example.com", otp_secret_key: "6iispf5cjufa4vsm") }
+
+ before do
+ sign_in user
+ end
+
+ context "with json" do
+ context "with totp authentication" do
+ it "returns 401 when path requires authentication" do
+ get "/dashboard.json"
+ expect(response.response_code).to eq(401)
+ body = JSON.parse(response.body)
+ expect(body["redirect_to"]).to eq(user_two_factor_authentication_path)
+ expect(body["authentication_type"]).to eq("totp")
+ end
+ end
+
+ context "with direct otp authentication" do
+ it "returns 401 when path requires authentication" do
+ user.update!(direct_otp: true)
+ get "/dashboard.json"
+ expect(response.response_code).to eq(401)
+ body = JSON.parse(response.body)
+ expect(body["redirect_to"]).to eq(user_two_factor_authentication_path)
+ expect(body["authentication_type"]).to eq("otp")
+ end
+ end
+
+ context "after TFA" do
+ it "returns successfully" do
+ get "/dashboard.json"
+ expect(response.response_code).to eq(401)
+ expect(JSON.parse(response.body)["redirect_to"]).to eq(user_two_factor_authentication_path)
+ totp_code = ROTP::TOTP.new(user.otp_secret_key, digits: 6).at(Time.now)
+ put "/users/two_factor_authentication", params: { code: totp_code }
+ get "/dashboard.json"
+ expect(response.response_code).to eq(200)
+ body = JSON.parse(response.body)
+ expect(body["success"]).to eq(true)
+ end
+ end
+ end
+
+ context "with xml" do
+ it "returns 401 when path requires authentication" do
+ get "/dashboard.xml"
+ expect(response.response_code).to eq(401)
+ end
+
+ context "after TFA" do
+ it "returns successfully" do
+ totp_code = ROTP::TOTP.new(user.otp_secret_key, digits: 6).at(Time.now)
+ put "/users/two_factor_authentication", params: { code: totp_code }
+ get "/dashboard.xml"
+ expect(response.response_code).to eq(200)
+ expect(response.body).to eq("")
+ end
+ end
+ end
+ end
+end
\ No newline at end of file
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 63704333..02ac6723 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -21,6 +21,8 @@
config.order = 'random'
config.after(:each) { Timecop.return }
+
+ config.include Devise::Test::IntegrationHelpers, type: :request
end
Dir["#{Dir.pwd}/spec/support/**/*.rb"].each {|f| require f}