Fix for ignored redirect on expired passwords. (ValiMail#13)

* [PLAT-484] Fix skip_current_devise_controller? to consider current class ancestors.

* [PLAT-484] Fix skip_current_controller? to consider current class ancestors.

* [PLAT-484] Update test databases.

* [PLAT-484] Fix require_no_authentication comments for clarity.

* [PLAT-484] Prevent redirect loops for DeviseInvitable.

* [PLAT-484] Calculate password freshness from updated_at instead of created_at.

If password_previously_used_count = 1, then the previous password will be updated, not created.

* [PLAT-484] Remove deprecated Gemfile.lock.ci files.

* [PLAT-484] Update gitignore to ignore *.gemfile.lock files.

* [PLAT-484] Update specs for revised password freshness algo.

* [PLAT-484] Update test databases.

* [PLAT-484] Bump version to 1.0.4.

Author: markeissler

.gitignore

@@ -52,6 +52,9 @@ Gemfile.lock
 .ruby-version
 .ruby-gemset
 
+# Ignore gemfiles/*.gemfile.lock files
+/gemfiles/*.gemfile.lock
+
 # unless supporting rvm < 1.11.0 or doing something fancy, ignore this:
 .rvmrc
 

Gemfile.lock.ci

@@ -18,13 +18,18 @@ def redirected_in_session?
           warden.authenticated? && warden.session['secure_password_last_controller'] == 'Devise::SessionsController'
         end
 
+        # Prevent infinite loops and allow specified controllers to bypass.
+        # @NOTE: The ability to extend this list may be made public, in the
+        # future if that functionality is needed.
         def skip_current_controller?
           exclusion_list = [
             'Devise::SessionsController',
             'Devise::PasswordsWithPolicyController#edit',
-            'Devise::PasswordsWithPolicyController#update'
+            'Devise::PasswordsWithPolicyController#update',
+            'DeviseInvitable::RegistrationsController#edit',
+            'DeviseInvitable::RegistrationsController#update'
           ]
-          exclusion_list.select { |e| e == "#{self.class.name}#" + action_name || e == self.class.name.to_s }.empty?
+          !(exclusion_list.include?("#{self.class.name}#" + action_name) || (exclusion_list & self.class.ancestors.map(&:to_s)).any?)
         end
 
         def error_string_for_password_expired

lib/devise/secure_password/controllers/active_helpers.rb

@@ -10,9 +10,9 @@ class ::DeviseController
 
           protected
 
-          # Override the devise require_no_authentication before callback so users
-          # have to prevent authenticated users with expired passwords from
-          # escaping to other pages without first updating their passwords.
+          # Override the devise require_no_authentication before callback to
+          # prevent authenticated users with expired passwords from escaping to
+          # other pages without first updating their passwords.
           def require_no_authentication
             return if check_password_expired_and_redirect!
 
@@ -42,11 +42,14 @@ def save_controller_state
             warden.session(scope_name)[:secure_last_action] = action_name
           end
 
+          # Prevent infinite loops and allow specified controllers to bypass.
+          # @NOTE: The ability to extend this list may be made public, in the
+          # future if that functionality is needed.
           def skip_current_devise_controller?
             exclusion_list = [
               'Devise::SessionsController'
             ]
-            exclusion_list.select { |e| e == "#{self.class.name}#" + action_name || e == self.class.name.to_s }.empty?
+            !(exclusion_list.include?("#{self.class.name}#" + action_name) || (exclusion_list & self.class.ancestors.map(&:to_s)).any?)
           end
 
           def error_string_for_password_expired

lib/devise/secure_password/controllers/devise_helpers.rb

@@ -9,11 +9,11 @@ class PreviousPassword < ::ActiveRecord::Base
       validates :encrypted_password, presence: true
 
       def fresh?(minimum_age_duration, now = ::Time.zone.now)
-        now <= (created_at + minimum_age_duration)
+        now <= (updated_at + minimum_age_duration)
       end
 
       def stale?(maximum_age_duration, now = ::Time.zone.now)
-        now > (created_at + maximum_age_duration)
+        now > (updated_at + maximum_age_duration)
       end
     end
   end

lib/devise/secure_password/models/previous_password.rb

@@ -1,5 +1,5 @@
 module Devise
   module SecurePassword
-    VERSION = '1.0.3'.freeze
+    VERSION = '1.0.4'.freeze
   end
 end

lib/devise/secure_password/version.rb

@@ -45,7 +45,7 @@
     before do
       user.save
       last_password = user.previous_passwords.unscoped.last
-      last_password.created_at = Time.zone.now - 2.days
+      last_password.created_at = last_password.updated_at = Time.zone.now - 2.days
       last_password.save
       login_as(user, scope: :user)
       visit '/users/change_password/edit'

spec/feature/user_changes_password_spec.rb

@@ -14,7 +14,7 @@
     before do
       user.save
       last_password = user.previous_passwords.unscoped.last
-      last_password.created_at = Time.zone.now - User.password_maximum_age
+      last_password.created_at = last_password.updated_at = Time.zone.now - User.password_maximum_age
       last_password.save
       visit '/users/sign_in'
     end

spec/feature/user_logs_in_spec.rb

@@ -62,7 +62,7 @@
         user.save
         # reset its previous_password record to one day before
         last_password = user.previous_passwords.unscoped.last
-        last_password.created_at = Time.zone.now - Isolated::UserFrequentChanges.password_minimum_age
+        last_password.created_at = last_password.updated_at = Time.zone.now - Isolated::UserFrequentChanges.password_minimum_age
         last_password.save
         # bypass frequent_reuse validator by changing password
         user.password = user.password_confirmation = user.password + 'Z'
@@ -89,7 +89,7 @@
         user.save
         # reset its previous_password record one day past minimum
         last_password = user.previous_passwords.unscoped.last
-        last_password.created_at = Time.zone.now - Isolated::UserFrequentChanges.password_minimum_age
+        last_password.created_at = last_password.updated_at = Time.zone.now - Isolated::UserFrequentChanges.password_minimum_age
         last_password.save
       end
 

spec/models/password_disallows_frequent_changes_spec.rb

@@ -41,7 +41,7 @@
         user.save
         # reset its previous_password record to one day past maximum
         last_password = user.previous_passwords.unscoped.last
-        last_password.created_at = Time.zone.now - Isolated::UserRegularUpdates.password_maximum_age
+        last_password.created_at = last_password.updated_at = Time.zone.now - Isolated::UserRegularUpdates.password_maximum_age
         last_password.save
       end
 

spec/models/password_requires_regular_updates_spec.rb

@@ -71,7 +71,7 @@
 
     context 'when a password is not recent' do
       it 'returns false' do
-        previous_password.created_at = Time.zone.now - 1.day
+        previous_password.created_at = previous_password.updated_at = Time.zone.now - 1.day
         expect(previous_password.fresh?(1.day)).to be false
       end
     end
@@ -92,7 +92,7 @@
 
     context 'when a password is old' do
       it 'returns true' do
-        previous_password.created_at = Time.zone.now - 1.day
+        previous_password.created_at = previous_password.updated_at = Time.zone.now - 1.day
         expect(previous_password.stale?(1.day)).to be true
       end
     end