Fix corner case when confirmation_sent_at is equal to 0.days.ago (#4529)

If `Confirmable#confirmation_sent_at` is equal to `0.days.ago`, then `confirmation_period_valid?` will be deemed valid even if the setting is configured to disable this outright. To prevent this error, we explicitly check the configuration setting to be `0.days.ago`.
heartcombo · Dec 4, 2018 · 1192c76 · 1192c76
1 parent ecd2187
commit 1192c76
Show file tree

Hide file tree

Showing 13 changed files with 31 additions and 2 deletions.
diff --git a/Gemfile b/Gemfile
@@ -19,6 +19,7 @@ gem "responders", "~> 2.4"
 group :test do
   gem "omniauth-facebook"
   gem "omniauth-openid"
+  gem "timecop"
   gem "webrat", "0.7.3", require: false
   gem "mocha", "~> 1.1", require: false
 end

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -164,6 +164,7 @@ GEM
     sqlite3 (1.3.13)
     thor (0.20.0)
     thread_safe (0.3.6)
+    timecop (0.8.1)
     tzinfo (1.2.5)
       thread_safe (~> 0.1)
     warden (1.2.7)
@@ -196,6 +197,7 @@ DEPENDENCIES
   rdoc
   responders (~> 2.4)
   sqlite3
+  timecop
   webrat (= 0.7.3)
 
 BUNDLED WITH

diff --git a/gemfiles/Gemfile.rails-4.1-stable b/gemfiles/Gemfile.rails-4.1-stable
@@ -12,6 +12,7 @@ gem "rdoc", "~> 5.1"
 group :test do
   gem "omniauth-facebook"
   gem "omniauth-openid"
+  gem "timecop"
   gem "webrat", "0.7.3", require: false
   gem "mocha", "~> 1.1", require: false
   gem 'test_after_commit', require: false

diff --git a/gemfiles/Gemfile.rails-4.1-stable.lock b/gemfiles/Gemfile.rails-4.1-stable.lock
@@ -137,6 +137,7 @@ GEM
     test_after_commit (1.1.0)
       activerecord (>= 3.2)
     thor (0.19.4)
+    timecop (0.8.1)
     thread_safe (0.3.6)
     tzinfo (1.2.2)
       thread_safe (~> 0.1)
@@ -165,6 +166,7 @@ DEPENDENCIES
   rdoc (~> 5.1)
   sqlite3
   test_after_commit
+  timecop
   webrat (= 0.7.3)
 
 BUNDLED WITH

diff --git a/gemfiles/Gemfile.rails-4.2-stable b/gemfiles/Gemfile.rails-4.2-stable
@@ -12,6 +12,7 @@ gem "rdoc", "~> 5.1"
 group :test do
   gem "omniauth-facebook"
   gem "omniauth-openid"
+  gem "timecop"
   gem "webrat", "0.7.3", require: false
   gem "mocha", "~> 1.1", require: false
   gem 'test_after_commit', require: false

diff --git a/gemfiles/Gemfile.rails-4.2-stable.lock b/gemfiles/Gemfile.rails-4.2-stable.lock
@@ -159,6 +159,7 @@ GEM
       activerecord (>= 3.2)
     thor (0.19.4)
     thread_safe (0.3.6)
+    timecop (0.8.1)
     tzinfo (1.2.2)
       thread_safe (~> 0.1)
     warden (1.2.7)
@@ -186,6 +187,7 @@ DEPENDENCIES
   rdoc (~> 5.1)
   sqlite3
   test_after_commit
+  timecop
   webrat (= 0.7.3)
 
 BUNDLED WITH

diff --git a/gemfiles/Gemfile.rails-5.0-stable b/gemfiles/Gemfile.rails-5.0-stable
@@ -18,6 +18,7 @@ gem "responders", "~> 2.1"
 group :test do
   gem "omniauth-facebook"
   gem "omniauth-openid"
+  gem "timecop"
   gem "webrat", "0.7.3", require: false
   gem "mocha", "~> 1.1", require: false
   gem 'test_after_commit', require: false

diff --git a/gemfiles/Gemfile.rails-5.0-stable.lock b/gemfiles/Gemfile.rails-5.0-stable.lock
@@ -157,6 +157,7 @@ GEM
       activerecord (>= 3.2)
     thor (0.19.4)
     thread_safe (0.3.6)
+    timecop (0.8.1)
     tzinfo (1.2.3)
       thread_safe (~> 0.1)
     warden (1.2.7)
@@ -186,6 +187,7 @@ DEPENDENCIES
   responders (~> 2.1)
   sqlite3
   test_after_commit
+  timecop
   webrat (= 0.7.3)
 
 BUNDLED WITH

diff --git a/gemfiles/Gemfile.rails-5.2-rc1 b/gemfiles/Gemfile.rails-5.2-rc1
@@ -16,6 +16,7 @@ gem "responders", "~> 2.1"
 group :test do
   gem "omniauth-facebook"
   gem "omniauth-openid"
+  gem "timecop"
   gem "webrat", "0.7.3", require: false
   gem "mocha", "~> 1.1", require: false
   gem 'test_after_commit', require: false

diff --git a/gemfiles/Gemfile.rails-5.2-rc1.lock b/gemfiles/Gemfile.rails-5.2-rc1.lock
@@ -166,6 +166,7 @@ GEM
       activerecord (>= 3.2)
     thor (0.20.0)
     thread_safe (0.3.6)
+    timecop (0.9.1)
     tzinfo (1.2.5)
       thread_safe (~> 0.1)
     warden (1.2.7)
@@ -195,7 +196,8 @@ DEPENDENCIES
   responders (~> 2.1)
   sqlite3
   test_after_commit
+  timecop
   webrat (= 0.7.3)
 
 BUNDLED WITH
-   1.16.1
+   1.17.1
diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb
@@ -211,7 +211,10 @@ def confirmation_required?
         #   confirmation_period_valid?   # will always return true
         #
         def confirmation_period_valid?
-          self.class.allow_unconfirmed_access_for.nil? || (confirmation_sent_at && confirmation_sent_at.utc >= self.class.allow_unconfirmed_access_for.ago)
+          return true if self.class.allow_unconfirmed_access_for.nil?
+          return false if self.class.allow_unconfirmed_access_for == 0.days
+
+          confirmation_sent_at && confirmation_sent_at.utc >= self.class.allow_unconfirmed_access_for.ago
         end
 
         # Checks if the user confirmation happens before the token becomes invalid

diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb
@@ -240,6 +240,16 @@ def setup
     refute user.active_for_authentication?
   end
 
+  test 'should not be active when confirm period is set to 0 days' do
+    Devise.allow_unconfirmed_access_for = 0.days
+    user = create_user
+
+    Timecop.freeze(Time.zone.today) do
+      user.confirmation_sent_at = Time.zone.today
+      refute user.active_for_authentication?
+    end
+  end
+
   test 'should be active when we set allow_unconfirmed_access_for to nil' do
     swap Devise, allow_unconfirmed_access_for: nil do
       user = create_user

diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -13,6 +13,7 @@
 I18n.load_path << File.expand_path("../support/locale/en.yml", __FILE__)
 
 require 'mocha/setup'
+require 'timecop'
 require 'webrat'
 Webrat.configure do |config|
   config.mode = :rails