From 31801fc9a0774657b30b08208ca54f9572609fbd Mon Sep 17 00:00:00 2001
From: Ashley Foster <ashley@ashleyfoster.me>
Date: Tue, 28 Nov 2017 09:58:41 -0500
Subject: [PATCH] Fix missing validations on Signup (#4674)

* Fix missing validations on Signup

This commit fixes issue
https://github.com/plataformatec/devise/issues/4673

This removes `validate: false` from saving a record when `Trackable` is
in use.

* Add test case

* Add mongoid model
---
 lib/devise/models/trackable.rb                |  2 +-
 test/models/trackable_test.rb                 |  9 +++++
 test/rails_app/app/active_record/user.rb      |  2 ++
 .../active_record/user_with_validations.rb    | 10 ++++++
 .../app/mongoid/user_with_validations.rb      | 35 +++++++++++++++++++
 test/support/helpers.rb                       |  4 +++
 6 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 test/rails_app/app/active_record/user_with_validations.rb
 create mode 100644 test/rails_app/app/mongoid/user_with_validations.rb

diff --git a/lib/devise/models/trackable.rb b/lib/devise/models/trackable.rb
index a5965235..adee0eaa 100644
--- a/lib/devise/models/trackable.rb
+++ b/lib/devise/models/trackable.rb
@@ -30,7 +30,7 @@ module Devise
 
       def update_tracked_fields!(request)
         update_tracked_fields(request)
-        save(validate: false)
+        save
       end
     end
   end
diff --git a/test/models/trackable_test.rb b/test/models/trackable_test.rb
index e792e337..07ac9802 100644
--- a/test/models/trackable_test.rb
+++ b/test/models/trackable_test.rb
@@ -38,4 +38,13 @@ class TrackableTest < ActiveSupport::TestCase
     assert_nil user.last_sign_in_at
     assert_equal 0, user.sign_in_count
   end
+
+  test 'update_tracked_fields should run model validations' do
+    user = UserWithValidations.new
+    request = mock
+    request.stubs(:remote_ip).returns("127.0.0.1")
+
+    assert_not user.update_tracked_fields!(request)
+    assert_not user.persisted?
+  end
 end
diff --git a/test/rails_app/app/active_record/user.rb b/test/rails_app/app/active_record/user.rb
index 78b3530a..1c9ebd41 100644
--- a/test/rails_app/app/active_record/user.rb
+++ b/test/rails_app/app/active_record/user.rb
@@ -4,4 +4,6 @@ class User < ActiveRecord::Base
   include Shim
   include SharedUser
   include ActiveModel::Serializers::Xml if Devise::Test.rails5?
+
+  validates :sign_in_count, presence: true
 end
diff --git a/test/rails_app/app/active_record/user_with_validations.rb b/test/rails_app/app/active_record/user_with_validations.rb
new file mode 100644
index 00000000..abfbe7e0
--- /dev/null
+++ b/test/rails_app/app/active_record/user_with_validations.rb
@@ -0,0 +1,10 @@
+require 'shared_user'
+
+class UserWithValidations < ActiveRecord::Base
+  self.table_name = 'users'
+  include Shim
+  include SharedUser
+
+  validates :email, presence: true
+end
+
diff --git a/test/rails_app/app/mongoid/user_with_validations.rb b/test/rails_app/app/mongoid/user_with_validations.rb
new file mode 100644
index 00000000..25dc5a31
--- /dev/null
+++ b/test/rails_app/app/mongoid/user_with_validations.rb
@@ -0,0 +1,35 @@
+require "shared_user"
+
+class UserWithValidations
+  include Mongoid::Document
+  include Shim
+  include SharedUser
+
+  field :username, type: String
+  field :facebook_token, type: String
+
+  ## Database authenticatable
+  field :email, type: String, default: ""
+  field :encrypted_password, type: String, default: ""
+
+  ## Recoverable
+  field :reset_password_token, type: String
+  field :reset_password_sent_at, type: Time
+
+  ## Rememberable
+  field :remember_created_at, type: Time
+
+  ## Trackable
+  field :sign_in_count, type: Integer, default: 0
+  field :current_sign_in_at, type: Time
+  field :last_sign_in_at, type: Time
+  field :current_sign_in_ip, type: String
+  field :last_sign_in_ip, type: String
+
+  ## Lockable
+  field :failed_attempts, type: Integer, default: 0 # Only if lock strategy is :failed_attempts
+  field :unlock_token, type: String # Only if unlock strategy is :email or :both
+  field :locked_at, type: Time
+
+  validates :email, presence: true
+end
diff --git a/test/support/helpers.rb b/test/support/helpers.rb
index ab77b91f..db82dd7c 100644
--- a/test/support/helpers.rb
+++ b/test/support/helpers.rb
@@ -50,6 +50,10 @@ class ActiveSupport::TestCase
     UserWithoutEmail.create!(valid_attributes(attributes))
   end
 
+  def create_user_with_validations(attributes={})
+    UserWithValidations.create!(valid_attributes(attributes))
+  end
+
   # Execute the block setting the given values and restoring old values after
   # the block is executed.
   def swap(object, new_values)