wip

Add assert to Lockable integration test and Replace mathcer `assert_not` to `refute`

.travis.yml

@@ -64,8 +64,6 @@ matrix:
 services:
   - mongodb
 
-sudo: false
-
 cache: bundler
 
 env:

CHANGELOG.md

@@ -1,4 +1,12 @@
 ### Unreleased
+* enhancements
+  * Increase default stretches to 12 (by @sergey-alekseev)
+
+### 4.7.1 - 2019-09-06
+
+* bug fixes
+  * Fix an edge case where records with a blank `confirmation_token` could be confirmed (by @tegon)
+  * Fix typo inside `update_needs_confirmation` i18n key (by @lslm)
 
 ### 4.7.0 - 2019-08-19
 

Gemfile.lock

@@ -10,7 +10,7 @@ GIT
 PATH
   remote: .
   specs:
-    devise (4.7.0)
+    devise (4.7.1)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
       railties (>= 4.1.0)

README.md

@@ -270,7 +270,7 @@ member_session
 The Devise method in your models also accepts some options to configure its modules. For example, you can choose the cost of the hashing algorithm with:
 
 ```ruby
-devise :database_authenticatable, :registerable, :confirmable, :recoverable, stretches: 12
+devise :database_authenticatable, :registerable, :confirmable, :recoverable, stretches: 13
 ```
 
 Besides `:stretches`, you can define `:pepper`, `:encryptor`, `:confirm_within`, `:remember_for`, `:timeout_in`, `:unlock_in` among other options. For more details, see the initializer file that was created when you invoked the "devise:install" generator described above. This file is usually located at `/config/initializers/devise.rb`.
@@ -289,7 +289,7 @@ There are just three actions in Devise that allow any set of parameters to be pa
 * `sign_up` (`Devise::RegistrationsController#create`) - Permits authentication keys plus `password` and `password_confirmation`
 * `account_update` (`Devise::RegistrationsController#update`) - Permits authentication keys plus `password`, `password_confirmation` and `current_password`
 
-In case you want to permit additional parameters (the lazy way™), you can do so using a simple before filter in your `ApplicationController`:
+In case you want to permit additional parameters (the lazy way™), you can do so using a simple before action in your `ApplicationController`:
 
 ```ruby
 class ApplicationController < ActionController::Base

app/controllers/devise_controller.rb

@@ -112,7 +112,7 @@ MESSAGE
     end
 
     if authenticated && resource = warden.user(resource_name)
-      flash[:alert] = I18n.t("devise.failure.already_authenticated")
+      set_flash_message(:alert, 'already_authenticated', scope: 'devise.failure')
       redirect_to after_sign_in_path_for(resource)
     end
   end

config/locales/en.yml

@@ -42,7 +42,7 @@ en:
       signed_up_but_inactive: "You have signed up successfully. However, we could not sign you in because your account is not yet activated."
       signed_up_but_locked: "You have signed up successfully. However, we could not sign you in because your account is locked."
       signed_up_but_unconfirmed: "A message with a confirmation link has been sent to your email address. Please follow the link to activate your account."
-      update_needs_confirmation: "You updated your account successfully, but we need to verify your new email address. Please check your email and follow the confirm link to confirm your new email address."
+      update_needs_confirmation: "You updated your account successfully, but we need to verify your new email address. Please check your email and follow the confirmation link to confirm your new email address."
       updated: "Your account has been updated successfully."
       updated_but_not_signed_in: "Your account has been updated successfully, but since your password was changed, you need to sign in again"
     sessions:

gemfiles/Gemfile.rails-4.1-stable.lock

@@ -21,10 +21,10 @@ GIT
 PATH
   remote: ..
   specs:
-    devise (4.6.2)
+    devise (4.7.1)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
-      railties (>= 4.1.0, < 6.0)
+      railties (>= 4.1.0)
       responders
       warden (~> 1.2.3)
 
@@ -54,7 +54,7 @@ GEM
       thread_safe (~> 0.1)
       tzinfo (~> 1.1)
     arel (5.0.1.20140414130214)
-    bcrypt (3.1.12)
+    bcrypt (3.1.13)
     bson (3.2.6)
     builder (3.2.3)
     concurrent-ruby (1.0.5)

gemfiles/Gemfile.rails-4.2-stable.lock

@@ -57,10 +57,10 @@ GIT
 PATH
   remote: ..
   specs:
-    devise (4.6.2)
+    devise (4.7.1)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
-      railties (>= 4.1.0, < 6.0)
+      railties (>= 4.1.0)
       responders
       warden (~> 1.2.3)
 
@@ -68,7 +68,7 @@ GEM
   remote: https://rubygems.org/
   specs:
     arel (6.0.4)
-    bcrypt (3.1.12)
+    bcrypt (3.1.13)
     bson (3.2.6)
     builder (3.2.3)
     concurrent-ruby (1.0.5)

gemfiles/Gemfile.rails-5.0-stable.lock

@@ -10,7 +10,7 @@ GIT
 PATH
   remote: ..
   specs:
-    devise (4.7.0)
+    devise (4.7.1)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
       railties (>= 4.1.0)
@@ -191,4 +191,4 @@ DEPENDENCIES
   webrat (= 0.7.3)
 
 BUNDLED WITH
-   1.17.1
+   1.17.3

gemfiles/Gemfile.rails-5.2-stable.lock

@@ -10,7 +10,7 @@ GIT
 PATH
   remote: ..
   specs:
-    devise (4.7.0)
+    devise (4.7.1)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
       railties (>= 4.1.0)
@@ -200,4 +200,4 @@ DEPENDENCIES
   webrat (= 0.7.3)
 
 BUNDLED WITH
-   1.17.1
+   1.17.3

gemfiles/Gemfile.rails-6.0-stable.lock

@@ -10,7 +10,7 @@ GIT
 PATH
   remote: ..
   specs:
-    devise (4.7.0)
+    devise (4.7.1)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
       railties (>= 4.1.0)
@@ -216,4 +216,4 @@ DEPENDENCIES
   webrat (= 0.7.3)
 
 BUNDLED WITH
-   1.17.2
+   1.17.3

lib/devise.rb

@@ -71,7 +71,7 @@ module Devise
 
   # The number of times to hash the password.
   mattr_accessor :stretches
-  @@stretches = 11
+  @@stretches = 12
 
   # The default key used when authenticating over http auth.
   mattr_accessor :http_authentication_key
@@ -305,6 +305,10 @@ module Devise
     defined?(ActiveRecord) && ActiveRecord.gem_version >= Gem::Version.new("5.1.x")
   end
 
+  def self.rails6_and_up?
+    Rails.gem_version >= Gem::Version.new("6.0.x")
+  end
+
   # Default way to set up Devise. Run rails generate devise_install to create
   # a fresh initializer with all configuration values.
   def self.setup

lib/devise/models/authenticatable.rb

@@ -152,7 +152,8 @@ module Devise
       #         # If the record is new or changed then delay the
       #         # delivery until the after_commit callback otherwise
       #         # send now because after_commit will not be called.
-      #         if new_record? || changed?
+      #         # For Rails < 6 is `changed?` instead of `saved_changes?`.
+      #         if new_record? || saved_changes?
       #           pending_devise_notifications << [notification, args]
       #         else
       #           render_and_send_devise_message(notification, *args)

lib/devise/models/confirmable.rb

@@ -348,7 +348,19 @@ module Devise
         # If the user is already confirmed, create an error for the user
         # Options must have the confirmation_token
         def confirm_by_token(confirmation_token)
+          # When the `confirmation_token` parameter is blank, if there are any users with a blank
+          # `confirmation_token` in the database, the first one would be confirmed here.
+          # The error is being manually added here to ensure no users are confirmed by mistake.
+          # This was done in the model for convenience, since validation errors are automatically
+          # displayed in the view.
+          if confirmation_token.blank?
+            confirmable = new
+            confirmable.errors.add(:confirmation_token, :blank)
+            return confirmable
+          end
+
           confirmable = find_first_by_auth_conditions(confirmation_token: confirmation_token)
+
           unless confirmable
             confirmation_digest = Devise.token_generator.digest(self, :confirmation_token, confirmation_token)
             confirmable = find_or_initialize_with_error_by(:confirmation_token, confirmation_digest)

lib/devise/rails.rb

@@ -43,5 +43,11 @@ module Devise
           )
         end
     end
+
+    initializer "devise.zeitwerk" do
+      if Devise.rails6_and_up? && Rails.autoloaders.zeitwerk_enabled? && !Object.const_defined?(Devise.parent_mailer)
+        Rails.autoloaders.main.ignore("#{__dir__}/app/mailers/devise/mailer.rb")
+      end
+    end
   end
 end

lib/devise/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Devise
-  VERSION = "4.7.0".freeze
+  VERSION = "4.7.1".freeze
 end

lib/generators/devise/views_generator.rb

@@ -42,7 +42,7 @@ module Devise
       def view_directory(name, _target_path = nil)
         directory name.to_s, _target_path || "#{target_path}/#{name}" do |content|
           if scope
-            content.gsub "devise/shared/links", "#{plural_scope}/shared/links"
+            content.gsub("devise/shared/links", "#{plural_scope}/shared/links").gsub("devise/shared/error_messages", "#{plural_scope}/shared/error_messages")
           else
             content
           end

lib/generators/templates/devise.rb

@@ -1,5 +1,11 @@
 # frozen_string_literal: true
 
+# Assuming you have not yet modified this file, each configuration option below
+# is set to its default value. Note that some are commented out while others
+# are not: uncommented lines are intended to protect your configuration from
+# breaking changes in upgrades (i.e., in the event that future versions of
+# Devise change the default values for those options).
+#
 # Use this hook to configure devise mailer, warden hooks and so forth.
 # Many of these configuration options can be set straight in your model.
 Devise.setup do |config|
@@ -103,7 +109,7 @@ Devise.setup do |config|
   # config.reload_routes = true
 
   # ==> Configuration for :database_authenticatable
-  # For bcrypt, this is the cost for hashing the password and defaults to 11. If
+  # For bcrypt, this is the cost for hashing the password and defaults to 12. If
   # using other algorithms, it sets how many times you want the password to be hashed.
   #
   # Limiting the stretches to just one in testing will increase the performance of
@@ -111,7 +117,7 @@ Devise.setup do |config|
   # a value less than 10 in other environments. Note that, for bcrypt (the default
   # algorithm), the cost increases exponentially with the number of stretches (e.g.
   # a value of 20 is already extremely slow: approx. 60 seconds for 1 calculation).
-  config.stretches = Rails.env.test? ? 1 : 11
+  config.stretches = Rails.env.test? ? 1 : 12
 
   # Set up a pepper to generate the hashed password.
   # config.pepper = '<%= SecureRandom.hex(64) %>'

test/generators/views_generator_test.rb

@@ -11,16 +11,19 @@ class ViewsGeneratorTest < Rails::Generators::TestCase
     run_generator
     assert_files
     assert_shared_links
+    assert_error_messages
   end
 
   test "Assert all views are properly created with scope param" do
     run_generator %w(users)
     assert_files "users"
     assert_shared_links "users"
+    assert_error_messages "users"
 
     run_generator %w(admins)
     assert_files "admins"
     assert_shared_links "admins"
+    assert_error_messages "admins"
   end
 
   test "Assert views with simple form" do
@@ -88,6 +91,7 @@ class ViewsGeneratorTest < Rails::Generators::TestCase
     assert_file "app/views/#{scope}/registrations/edit.html.erb"
     assert_file "app/views/#{scope}/sessions/new.html.erb"
     assert_file "app/views/#{scope}/shared/_links.html.erb"
+    assert_file "app/views/#{scope}/shared/_error_messages.html.erb"
     assert_file "app/views/#{scope}/unlocks/new.html.erb"
   end
 
@@ -102,4 +106,16 @@ class ViewsGeneratorTest < Rails::Generators::TestCase
     assert_file "app/views/#{scope}/sessions/new.html.erb", link
     assert_file "app/views/#{scope}/unlocks/new.html.erb", link
   end
+
+  def assert_error_messages(scope = nil)
+    scope = "devise" if scope.nil?
+    link = /<%= render \"#{scope}\/shared\/error_messages\", resource: resource %>/
+
+    assert_file "app/views/#{scope}/passwords/edit.html.erb", link
+    assert_file "app/views/#{scope}/passwords/new.html.erb", link
+    assert_file "app/views/#{scope}/confirmations/new.html.erb", link
+    assert_file "app/views/#{scope}/registrations/new.html.erb", link
+    assert_file "app/views/#{scope}/registrations/edit.html.erb", link
+    assert_file "app/views/#{scope}/unlocks/new.html.erb", link
+  end
 end

test/integration/authenticatable_test.rb

@@ -323,6 +323,14 @@ class AuthenticationRedirectTest < Devise::IntegrationTest
     visit new_user_session_path
     assert_equal flash[:alert], I18n.t("devise.failure.already_authenticated")
   end
+
+  test 'require_no_authentication should set the already_authenticated flash message as admin' do
+    store_translations :en, devise: { failure: { admin: { already_authenticated: 'You are already signed in as admin.' } } } do
+      sign_in_as_admin
+      visit new_admin_session_path
+      assert_equal flash[:alert], "You are already signed in as admin."
+    end
+  end
 end
 
 class AuthenticationSessionTest < Devise::IntegrationTest

test/integration/confirmable_test.rb

@@ -175,6 +175,36 @@ class ConfirmationTest < Devise::IntegrationTest
     assert_current_url '/users/sign_in'
   end
 
+  test "should not be able to confirm an email with a blank confirmation token" do
+    visit_user_confirmation_with_token("")
+
+    assert_contain "Confirmation token can't be blank"
+  end
+
+  test "should not be able to confirm an email with a nil confirmation token" do
+    visit_user_confirmation_with_token(nil)
+
+    assert_contain "Confirmation token can't be blank"
+  end
+
+  test "should not be able to confirm user with blank confirmation token" do
+    user = create_user(confirm: false)
+    user.update_attribute(:confirmation_token, "")
+
+    visit_user_confirmation_with_token("")
+
+    assert_contain "Confirmation token can't be blank"
+  end
+
+  test "should not be able to confirm user with nil confirmation token" do
+    user = create_user(confirm: false)
+    user.update_attribute(:confirmation_token, nil)
+
+    visit_user_confirmation_with_token(nil)
+
+    assert_contain "Confirmation token can't be blank"
+  end
+
   test 'error message is configurable by resource name' do
     store_translations :en, devise: {
       failure: { user: { unconfirmed: "Not confirmed user" } }

test/integration/trackable_test.rb

@@ -44,7 +44,7 @@ class TrackableHooksTest < Devise::IntegrationTest
     assert_equal "127.0.0.1", user.last_sign_in_ip
   end
 
-  test "current remote ip returns original ip behind a non transparent proxy" do
+  test "current and last sign in remote ip returns original ip behind a non transparent proxy" do
     user = create_user
 
     arbitrary_ip = '200.121.1.69'
@@ -53,6 +53,7 @@ class TrackableHooksTest < Devise::IntegrationTest
     end
     user.reload
     assert_equal arbitrary_ip, user.current_sign_in_ip
+    assert_equal arbitrary_ip, user.last_sign_in_ip
   end
 
   test "increase sign in count" do

test/models/confirmable_test.rb

@@ -77,6 +77,24 @@ class ConfirmableTest < ActiveSupport::TestCase
     assert_equal "can't be blank", confirmed_user.errors[:confirmation_token].join
   end
 
+  test 'should return a new record with errors when a blank token is given and a record exists on the database' do
+    user = create_user(confirmation_token: '')
+
+    confirmed_user = User.confirm_by_token('')
+
+    refute user.reload.confirmed?
+    assert_equal "can't be blank", confirmed_user.errors[:confirmation_token].join
+  end
+
+  test 'should return a new record with errors when a nil token is given and a record exists on the database' do
+    user = create_user(confirmation_token: nil)
+
+    confirmed_user = User.confirm_by_token(nil)
+
+    refute user.reload.confirmed?
+    assert_equal "can't be blank", confirmed_user.errors[:confirmation_token].join
+  end
+
   test 'should generate errors for a user email if user is already confirmed' do
     user = create_user
     user.confirmed_at = Time.now

test/rails_app/config/application.rb

@@ -49,5 +49,9 @@ module RailsApp
     if Devise::Test.rails52_and_up? && !Devise::Test.rails6?
       Rails.application.config.active_record.sqlite3.represent_boolean_as_integer = true
     end
+
+    if Devise.rails6_and_up?
+      config.autoloader = :zeitwerk
+    end
   end
 end

test/rails_app/config/initializers/devise.rb

@@ -3,6 +3,12 @@
 require "omniauth-facebook"
 require "omniauth-openid"
 
+# Assuming you have not yet modified this file, each configuration option below
+# is set to its default value. Note that some are commented out while others
+# are not: uncommented lines are intended to protect your configuration from
+# breaking changes in upgrades (i.e., in the event that future versions of
+# Devise change the default values for those options).
+#
 # Use this hook to configure devise mailer, warden hooks and so forth. The first
 # four configuration values can also be set straight in your models.
 Devise.setup do |config|

test/rails_test.rb

@@ -8,4 +8,24 @@ class RailsTest < ActiveSupport::TestCase
     assert_equal :load_config_initializers, initializer.after
     assert_equal :build_middleware_stack, initializer.before
   end
+
+  test 'ignore devise mailer loading when ActionMailer is not defined with zeitwerk' do
+    if Devise.rails6_and_up?
+      begin
+        swap Devise, parent_mailer: 'NotDefinedParentMailer' do
+          Devise::Engine.initializers.detect { |initializer| initializer.name == 'devise.zeitwerk' }.block.call
+          assert Rails.autoloaders.main.ignored_glob_patterns.any? { |pattern| pattern.include?("mailer.rb") }
+        end
+      ensure
+        Rails.autoloaders.main.instance_variable_set(:@ignored_glob_patterns, Set.new)
+      end
+    end
+  end
+
+  test 'load devise mailer file when Devise.parent_mailer is defined with zeitwerk' do
+    if Devise.rails6_and_up?
+      Devise::Engine.initializers.detect { |initializer| initializer.name == 'devise.zeitwerk' }.block.call
+      refute Rails.autoloaders.main.ignored_glob_patterns.any? { |pattern| pattern.include?("mailer.rb") }
+    end
+  end
 end