Browse Source

Fix class detection in reset_otp_state_for(user)

**Why**:
When looking up a class with `Object.const_defined?`, `false` will be
returned the first time it is called, even when the class exists in the
Rails app. I think this might be due to the way Rails loads classes.

**How**:
Use `ActiveSupport::Inflector#constantize`, which returns the class all
the time when the class exists, and throws a `NameError` when it
doesn't.

The only way I was able to properly test this was to create the
`UserOtpSender` class as a real file in the test Rails app, and create
a Devise Admin user to test the scenario where `AdminOtpSender` does
not exist.

I verified that with the old code, `reset_otp_state` was not being
called when it should be, and that the new code makes the tests pass.
master
Moncef Belyamani 9 years ago
parent
commit
157746a03e
  1. 10
      lib/two_factor_authentication/hooks/two_factor_authenticatable.rb
  2. 61
      spec/features/two_factor_authenticatable_spec.rb
  3. 6
      spec/rails_app/app/models/admin.rb
  4. 9
      spec/rails_app/app/services/user_otp_sender.rb
  5. 4
      spec/rails_app/config/initializers/devise.rb
  6. 1
      spec/rails_app/config/routes.rb
  7. 2
      spec/rails_app/db/migrate/20140403184646_devise_create_users.rb
  8. 42
      spec/rails_app/db/migrate/20160209032439_devise_create_admins.rb
  9. 20
      spec/rails_app/db/schema.rb
  10. 4
      spec/support/authenticated_model_helper.rb

10
lib/two_factor_authentication/hooks/two_factor_authenticatable.rb

@ -22,11 +22,17 @@ end
def reset_otp_state_for(user)
klass_string = "#{user.class}OtpSender"
return unless Object.const_defined?(klass_string)
klass = class_from_string(klass_string)
klass = Object.const_get(klass_string)
return unless klass
otp_sender = klass.new(user)
otp_sender.reset_otp_state if otp_sender.respond_to?(:reset_otp_state)
end
def class_from_string(string)
string.constantize
rescue NameError
false
end

61
spec/features/two_factor_authenticatable_spec.rb

@ -187,29 +187,16 @@ feature "User of two factor authentication" do
describe 'signing in' do
let(:user) { create_user }
let(:admin) { create_admin }
scenario 'when UserOtpSender#reset_otp_state is defined' do
klass = stub_const 'UserOtpSender', Class.new
klass.class_eval do
def reset_otp_state; end
end
otp_sender = instance_double(UserOtpSender)
expect(UserOtpSender).to receive(:new).with(user).and_return(otp_sender)
expect(otp_sender).to receive(:reset_otp_state)
visit new_user_session_path
complete_sign_in_form_for(user)
expect(user.reload.email).to eq 'updated@example.com'
end
scenario 'when UserOtpSender#reset_otp_state is not defined' do
klass = stub_const 'UserOtpSender', Class.new
klass.class_eval do
def reset_otp_state; end
end
otp_sender = instance_double(UserOtpSender)
allow(otp_sender).to receive(:respond_to?).with(:reset_otp_state).and_return(false)
@ -219,44 +206,34 @@ feature "User of two factor authentication" do
visit new_user_session_path
complete_sign_in_form_for(user)
end
scenario 'when AdminOtpSender is not defined' do
visit new_admin_session_path
complete_sign_in_form_for(admin)
expect(page).to have_content('Signed in successfully.')
end
end
describe 'signing out' do
let(:user) { create_user }
let(:admin) { create_admin }
scenario 'when UserOtpSender#reset_otp_state is defined' do
visit new_user_session_path
complete_sign_in_form_for(user)
klass = stub_const 'UserOtpSender', Class.new
klass.class_eval do
def reset_otp_state; end
end
otp_sender = instance_double(UserOtpSender)
expect(UserOtpSender).to receive(:new).with(user).and_return(otp_sender)
expect(otp_sender).to receive(:reset_otp_state)
user.update_attributes(email: 'foo@example.com')
visit destroy_user_session_path
end
scenario 'when UserOtpSender#reset_otp_state is not defined' do
visit new_user_session_path
complete_sign_in_form_for(user)
klass = stub_const 'UserOtpSender', Class.new
klass.class_eval do
def reset_otp_state; end
end
otp_sender = instance_double(UserOtpSender)
allow(otp_sender).to receive(:respond_to?).with(:reset_otp_state).and_return(false)
expect(user.reload.email).to eq 'updated@example.com'
end
expect(UserOtpSender).to receive(:new).with(user).and_return(otp_sender)
expect(otp_sender).to_not receive(:reset_otp_state)
scenario 'when AdminOtpSender is not defined' do
visit new_admin_session_path
complete_sign_in_form_for(admin)
visit destroy_admin_session_path
visit destroy_user_session_path
expect(page).to have_content('Signed out successfully.')
end
end
end

6
spec/rails_app/app/models/admin.rb

@ -0,0 +1,6 @@
class Admin < ActiveRecord::Base
# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable and :omniauthable
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :trackable, :validatable
end

9
spec/rails_app/app/services/user_otp_sender.rb

@ -0,0 +1,9 @@
class UserOtpSender
def initialize(user)
@user = user
end
def reset_otp_state
@user.update_attributes(email: 'updated@example.com')
end
end

4
spec/rails_app/config/initializers/devise.rb

@ -206,11 +206,11 @@ Devise.setup do |config|
# Configure the default scope given to Warden. By default it's the first
# devise role declared in your routes (usually :user).
# config.default_scope = :user
config.default_scope = :user
# Set this configuration to false if you want /users/sign_out to sign out
# only the current scope. By default, Devise signs out all scopes.
# config.sign_out_all_scopes = true
config.sign_out_all_scopes = false
# ==> Navigation configuration
# Lists the formats that should be treated as navigational. Formats like

1
spec/rails_app/config/routes.rb

@ -1,4 +1,5 @@
Dummy::Application.routes.draw do
devise_for :admins
root to: "home#index"
match "/dashboard", to: "home#dashboard", as: :dashboard, via: [:get]

2
spec/rails_app/db/migrate/20140403184646_devise_create_users.rb

@ -31,7 +31,7 @@ class DeviseCreateUsers < ActiveRecord::Migration
# t.datetime :locked_at
t.timestamps
t.timestamps null: false
end
add_index :users, :email, unique: true

42
spec/rails_app/db/migrate/20160209032439_devise_create_admins.rb

@ -0,0 +1,42 @@
class DeviseCreateAdmins < ActiveRecord::Migration
def change
create_table(:admins) do |t|
## Database authenticatable
t.string :email, null: false, default: ""
t.string :encrypted_password, null: false, default: ""
## Recoverable
t.string :reset_password_token
t.datetime :reset_password_sent_at
## Rememberable
t.datetime :remember_created_at
## Trackable
t.integer :sign_in_count, default: 0, null: false
t.datetime :current_sign_in_at
t.datetime :last_sign_in_at
t.string :current_sign_in_ip
t.string :last_sign_in_ip
## Confirmable
# t.string :confirmation_token
# t.datetime :confirmed_at
# t.datetime :confirmation_sent_at
# t.string :unconfirmed_email # Only if using reconfirmable
## Lockable
# t.integer :failed_attempts, default: 0, null: false # Only if lock strategy is :failed_attempts
# t.string :unlock_token # Only if unlock strategy is :email or :both
# t.datetime :locked_at
t.timestamps null: false
end
add_index :admins, :email, unique: true
add_index :admins, :reset_password_token, unique: true
# add_index :admins, :confirmation_token, unique: true
# add_index :admins, :unlock_token, unique: true
end
end

20
spec/rails_app/db/schema.rb

@ -11,7 +11,25 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20151228230340) do
ActiveRecord::Schema.define(version: 20160209032439) do
create_table "admins", force: :cascade do |t|
t.string "email", default: "", null: false
t.string "encrypted_password", default: "", null: false
t.string "reset_password_token"
t.datetime "reset_password_sent_at"
t.datetime "remember_created_at"
t.integer "sign_in_count", default: 0, null: false
t.datetime "current_sign_in_at"
t.datetime "last_sign_in_at"
t.string "current_sign_in_ip"
t.string "last_sign_in_ip"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end
add_index "admins", ["email"], name: "index_admins_on_email", unique: true
add_index "admins", ["reset_password_token"], name: "index_admins_on_reset_password_token", unique: true
create_table "users", force: :cascade do |t|
t.string "email", default: "", null: false

4
spec/support/authenticated_model_helper.rb

@ -9,6 +9,10 @@ module AuthenticatedModelHelper
User.create!(valid_attributes(attributes))
end
def create_admin
Admin.create!(valid_attributes.except(:nickname))
end
def valid_attributes(attributes={})
{
nickname: 'Marissa',

Loading…
Cancel
Save