From e0ff5e186e8a617f7085de21c7d21a26bf559cc2 Mon Sep 17 00:00:00 2001 From: Genaro Madrid Date: Wed, 29 Jun 2016 15:12:16 -0500 Subject: [PATCH 1/5] validate authentication for lockable option --- app/controllers/devise_token_auth/sessions_controller.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/controllers/devise_token_auth/sessions_controller.rb b/app/controllers/devise_token_auth/sessions_controller.rb index c85af0c77..86db56310 100644 --- a/app/controllers/devise_token_auth/sessions_controller.rb +++ b/app/controllers/devise_token_auth/sessions_controller.rb @@ -29,7 +29,12 @@ def create @resource = resource_class.where(q, q_value).first end - if @resource and valid_params?(field, q_value) and @resource.valid_password?(resource_params[:password]) and (!@resource.respond_to?(:active_for_authentication?) or @resource.active_for_authentication?) + if @resource and valid_params?(field, q_value) and (!@resource.respond_to?(:active_for_authentication?) or @resource.active_for_authentication?) + valid_password = @resource.valid_password?(resource_params[:password]) + if (@resource.respond_to?(:valid_for_authentication?) && !@resource.valid_for_authentication? { valid_password }) || !valid_password + render_create_error_bad_credentials + return + end # create client id @client_id = SecureRandom.urlsafe_base64(nil, false) @token = SecureRandom.urlsafe_base64(nil, false) From 19f11ee0fcfc79021b4dc2fdc58763b2b0ef710e Mon Sep 17 00:00:00 2001 From: Genaro Madrid Date: Wed, 29 Jun 2016 15:12:34 -0500 Subject: [PATCH 2/5] allow to overwrite unlocks controller --- lib/devise_token_auth/rails/routes.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/devise_token_auth/rails/routes.rb b/lib/devise_token_auth/rails/routes.rb index 880331f82..af1f5195d 100644 --- a/lib/devise_token_auth/rails/routes.rb +++ b/lib/devise_token_auth/rails/routes.rb @@ -12,6 +12,7 @@ def mount_devise_token_auth_for(resource, opts) confirmations_ctrl = opts[:controllers][:confirmations] || "devise_token_auth/confirmations" token_validations_ctrl = opts[:controllers][:token_validations] || "devise_token_auth/token_validations" omniauth_ctrl = opts[:controllers][:omniauth_callbacks] || "devise_token_auth/omniauth_callbacks" + unlocks_ctrl = opts[:controllers][:unlocks] # define devise controller mappings controllers = {:sessions => sessions_ctrl, @@ -19,6 +20,8 @@ def mount_devise_token_auth_for(resource, opts) :passwords => passwords_ctrl, :confirmations => confirmations_ctrl} + controllers[:unlocks] = unlocks_ctrl if unlocks_ctrl + # remove any unwanted devise modules opts[:skip].each{|item| controllers.delete(item)} From ab2cfad137ee99259e0294a31111ea6760815bbf Mon Sep 17 00:00:00 2001 From: Genaro Madrid Date: Wed, 29 Jun 2016 20:48:03 -0500 Subject: [PATCH 3/5] test lockable users --- .../sessions_controller_test.rb | 89 +++++++++++++++++++ test/dummy/app/models/lockable_user.rb | 5 ++ test/dummy/config/routes.rb | 2 + ...devise_token_auth_create_lockable_users.rb | 60 +++++++++++++ test/dummy/db/schema.rb | 22 ++++- test/fixtures/lockable_users.yml | 22 +++++ 6 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 test/dummy/app/models/lockable_user.rb create mode 100644 test/dummy/db/migrate/20160629184441_devise_token_auth_create_lockable_users.rb create mode 100644 test/fixtures/lockable_users.yml diff --git a/test/controllers/devise_token_auth/sessions_controller_test.rb b/test/controllers/devise_token_auth/sessions_controller_test.rb index 9875f650c..7dc9458b2 100644 --- a/test/controllers/devise_token_auth/sessions_controller_test.rb +++ b/test/controllers/devise_token_auth/sessions_controller_test.rb @@ -389,5 +389,94 @@ def @controller.reset_session; @reset_session_called = true; end refute OnlyEmailUser.method_defined?(:confirmed_at) end end + + describe "Lockable User" do + setup do + @request.env['devise.mapping'] = Devise.mappings[:lockable_user] + end + + teardown do + @request.env['devise.mapping'] = Devise.mappings[:user] + end + + before do + @original_lock_strategy = Devise.lock_strategy + @original_unlock_strategy = Devise.unlock_strategy + @original_maximum_attempts = Devise.maximum_attempts + Devise.lock_strategy = :failed_attempts + Devise.unlock_strategy = :email + Devise.maximum_attempts = 5 + end + + after do + Devise.lock_strategy = @original_lock_strategy + Devise.maximum_attempts = @original_maximum_attempts + Devise.unlock_strategy = @original_unlock_strategy + end + + describe "locked user" do + before do + @locked_user = lockable_users(:locked_user) + xhr :post, :create, { + email: @locked_user.email, + password: 'secret123' + } + @data = JSON.parse(response.body) + end + + test "request should fail" do + assert_equal 401, response.status + end + + test "response should contain errors" do + assert @data['errors'] + assert_equal @data['errors'], [I18n.t("devise_token_auth.sessions.not_confirmed", email: @locked_user.email)] + end + end + + describe "unlocked user with bad password" do + before do + @unlocked_user = lockable_users(:unlocked_user) + xhr :post, :create, { + email: @unlocked_user.email, + password: 'bad-password' + } + @data = JSON.parse(response.body) + end + + test "request should fail" do + assert_equal 401, response.status + end + + test "should increase failed_attempts" do + assert_equal 1, @unlocked_user.reload.failed_attempts + end + + test "response should contain errors" do + assert @data['errors'] + assert_equal @data['errors'], [I18n.t("devise_token_auth.sessions.bad_credentials")] + end + + describe 'after maximum_attempts should block the user' do + before do + 4.times do + xhr :post, :create, { + email: @unlocked_user.email, + password: 'bad-password' + } + end + @data = JSON.parse(response.body) + end + + test "should increase failed_attempts" do + assert_equal 5, @unlocked_user.reload.failed_attempts + end + + test "should block the user" do + assert_equal true, @unlocked_user.reload.access_locked? + end + end + end + end end end diff --git a/test/dummy/app/models/lockable_user.rb b/test/dummy/app/models/lockable_user.rb new file mode 100644 index 000000000..79e34a6b2 --- /dev/null +++ b/test/dummy/app/models/lockable_user.rb @@ -0,0 +1,5 @@ +class LockableUser < ActiveRecord::Base + # Include default devise modules. + devise :database_authenticatable, :registerable, :lockable + include DeviseTokenAuth::Concerns::User +end diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index b2f45c838..c0b6472ea 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -34,6 +34,8 @@ mount_devise_token_auth_for 'UnconfirmableUser', at: 'unconfirmable_user_auth' + mount_devise_token_auth_for 'LockableUser', at: 'lockable_user_auth' + # test namespacing namespace :api do scope :v1 do diff --git a/test/dummy/db/migrate/20160629184441_devise_token_auth_create_lockable_users.rb b/test/dummy/db/migrate/20160629184441_devise_token_auth_create_lockable_users.rb new file mode 100644 index 000000000..460c0a22d --- /dev/null +++ b/test/dummy/db/migrate/20160629184441_devise_token_auth_create_lockable_users.rb @@ -0,0 +1,60 @@ +include MigrationDatabaseHelper + +class DeviseTokenAuthCreateLockableUsers < ActiveRecord::Migration + def change + create_table(:lockable_users) do |t| + ## Required + t.string :provider, :null => false + t.string :uid, :null => false, :default => "" + + ## Database authenticatable + 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 + + ## User Info + t.string :name + t.string :nickname + t.string :image + t.string :email + + ## Tokens + if json_supported_database? + t.json :tokens + else + t.text :tokens + end + + t.timestamps + end + + add_index :lockable_users, :email + add_index :lockable_users, [:uid, :provider], :unique => true + # add_index :lockable_users, :reset_password_token, :unique => true + # add_index :lockable_users, :confirmation_token, :unique => true + add_index :lockable_users, :unlock_token, :unique => true + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index c5e24774c..6e5b299b1 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160103235141) do +ActiveRecord::Schema.define(version: 20160629184441) do create_table "evil_users", force: :cascade do |t| t.string "email" @@ -44,6 +44,26 @@ add_index "evil_users", ["reset_password_token"], name: "index_evil_users_on_reset_password_token", unique: true add_index "evil_users", ["uid", "provider"], name: "index_evil_users_on_uid_and_provider", unique: true + create_table "lockable_users", force: :cascade do |t| + t.string "provider", null: false + t.string "uid", default: "", null: false + t.string "encrypted_password", default: "", null: false + t.integer "failed_attempts", default: 0, null: false + t.string "unlock_token" + t.datetime "locked_at" + t.string "name" + t.string "nickname" + t.string "image" + t.string "email" + t.text "tokens" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "lockable_users", ["email"], name: "index_lockable_users_on_email" + add_index "lockable_users", ["uid", "provider"], name: "index_lockable_users_on_uid_and_provider", unique: true + add_index "lockable_users", ["unlock_token"], name: "index_lockable_users_on_unlock_token", unique: true + create_table "mangs", force: :cascade do |t| t.string "email" t.string "encrypted_password", default: "", null: false diff --git a/test/fixtures/lockable_users.yml b/test/fixtures/lockable_users.yml new file mode 100644 index 000000000..7a299b32f --- /dev/null +++ b/test/fixtures/lockable_users.yml @@ -0,0 +1,22 @@ +<% @locked_user = Faker::Internet.email %> +<% timestamp = DateTime.parse(1.day.ago.to_s).to_time.strftime("%F %T") %> +unlocked_user: + uid: "<%= @locked_user %>" + email: "<%= @locked_user %>" + provider: 'email' + created_at: '<%= timestamp %>' + updated_at: '<%= timestamp %>' + encrypted_password: <%= User.new.send(:password_digest, 'secret123') %> + +<% @locked_user = Faker::Internet.email %> +<% timestamp = DateTime.parse(1.day.ago.to_s).to_time.strftime("%F %T") %> +locked_user: + uid: "<%= @locked_user %>" + email: "<%= @locked_user %>" + provider: 'email' + created_at: '<%= timestamp %>' + updated_at: '<%= timestamp %>' + encrypted_password: <%= User.new.send(:password_digest, 'secret123') %> + locked_at: '<%= timestamp %>' + failed_attempts: 5 + From 567f792dd9c63215031d7da084535e9efd9d4351 Mon Sep 17 00:00:00 2001 From: Daniel Neis Araujo Date: Mon, 6 Nov 2017 18:05:53 -0200 Subject: [PATCH 4/5] Support authentication with multiple fields --- .../concerns/resource_finder.rb | 24 ++++++++++++++----- .../devise_token_auth/passwords_controller.rb | 2 +- .../devise_token_auth/sessions_controller.rb | 10 ++------ .../devise_token_auth/unlocks_controller.rb | 2 +- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/app/controllers/devise_token_auth/concerns/resource_finder.rb b/app/controllers/devise_token_auth/concerns/resource_finder.rb index 4103ae090..3cd19e656 100644 --- a/app/controllers/devise_token_auth/concerns/resource_finder.rb +++ b/app/controllers/devise_token_auth/concerns/resource_finder.rb @@ -12,14 +12,26 @@ def get_case_insensitive_field_from_resource_params(field) q_value end - def find_resource(field, value) - # fix for mysql default case insensitivity - q = "#{field.to_s} = ? AND provider='#{provider.to_s}'" - if ActiveRecord::Base.connection.adapter_name.downcase.starts_with? 'mysql' - q = "BINARY " + q + def find_resource + + fields = (resource_params.keys.map(&:to_sym) & resource_class.authentication_keys) + + conditions = [] + values = {} + fields.each do |f| + q = " #{f.to_s} = :#{f.to_s} " + # fix for mysql default case insensitivity + if ActiveRecord::Base.connection.adapter_name.downcase.starts_with? 'mysql' + q = "BINARY " + q + end + conditions.push(q) + values[f.to_sym] = get_case_insensitive_field_from_resource_params(f) end - @resource = resource_class.where(q, value).first + conditions.push(' provider = :provider') + values[:provider] = provider.to_s + + @resource = resource_class.where([conditions.join(" AND "), values]).first end def resource_class(m=nil) diff --git a/app/controllers/devise_token_auth/passwords_controller.rb b/app/controllers/devise_token_auth/passwords_controller.rb index 631dd7724..059a2887c 100644 --- a/app/controllers/devise_token_auth/passwords_controller.rb +++ b/app/controllers/devise_token_auth/passwords_controller.rb @@ -28,7 +28,7 @@ def create end @email = get_case_insensitive_field_from_resource_params(:email) - @resource = find_resource(:uid, @email) + @resource = find_resource @errors = nil @error_status = 400 diff --git a/app/controllers/devise_token_auth/sessions_controller.rb b/app/controllers/devise_token_auth/sessions_controller.rb index cebfa03c2..d8b68e433 100644 --- a/app/controllers/devise_token_auth/sessions_controller.rb +++ b/app/controllers/devise_token_auth/sessions_controller.rb @@ -10,16 +10,10 @@ def new def create # Check - field = (resource_params.keys.map(&:to_sym) & resource_class.authentication_keys).first - @resource = nil - if field - q_value = get_case_insensitive_field_from_resource_params(field) + @resource = find_resource - @resource = find_resource(field, q_value) - end - - if @resource && valid_params?(field, q_value) && (!@resource.respond_to?(:active_for_authentication?) || @resource.active_for_authentication?) + if @resource && (!@resource.respond_to?(:active_for_authentication?) || @resource.active_for_authentication?) valid_password = @resource.valid_password?(resource_params[:password]) if (@resource.respond_to?(:valid_for_authentication?) && !@resource.valid_for_authentication? { valid_password }) || !valid_password render_create_error_bad_credentials diff --git a/app/controllers/devise_token_auth/unlocks_controller.rb b/app/controllers/devise_token_auth/unlocks_controller.rb index 7286c899a..c643ba5b2 100644 --- a/app/controllers/devise_token_auth/unlocks_controller.rb +++ b/app/controllers/devise_token_auth/unlocks_controller.rb @@ -10,7 +10,7 @@ def create end @email = get_case_insensitive_field_from_resource_params(:email) - @resource = find_resource(:email, @email) + @resource = find_resource @errors = nil @error_status = 400 From 07d07acf0a76e643d6584432040cd7acbc51c5ab Mon Sep 17 00:00:00 2001 From: Daniel Neis Araujo Date: Fri, 6 Apr 2018 14:47:59 -0300 Subject: [PATCH 5/5] changes after revision from MaicolBen --- .../devise_token_auth/concerns/resource_finder.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/devise_token_auth/concerns/resource_finder.rb b/app/controllers/devise_token_auth/concerns/resource_finder.rb index 3cd19e656..88c5752da 100644 --- a/app/controllers/devise_token_auth/concerns/resource_finder.rb +++ b/app/controllers/devise_token_auth/concerns/resource_finder.rb @@ -18,20 +18,20 @@ def find_resource conditions = [] values = {} - fields.each do |f| - q = " #{f.to_s} = :#{f.to_s} " + fields.each do |field| + condition = " #{field.to_s} = :#{field.to_s} " # fix for mysql default case insensitivity if ActiveRecord::Base.connection.adapter_name.downcase.starts_with? 'mysql' - q = "BINARY " + q + condition = "BINARY " + condition end - conditions.push(q) - values[f.to_sym] = get_case_insensitive_field_from_resource_params(f) + conditions.push(condition) + values[field.to_sym] = get_case_insensitive_field_from_resource_params(field) end conditions.push(' provider = :provider') values[:provider] = provider.to_s - @resource = resource_class.where([conditions.join(" AND "), values]).first + @resource = resource_class.where([conditions.join(' AND '), values]).first end def resource_class(m=nil)