Add additional step for unlocking account

app/controllers/devise/unlocks_controller.rb

@@ -22,6 +22,38 @@ def create
 
   # GET /resource/unlock?unlock_token=abcdef
   def show
+    if resource_class.extra_step
+      @token = params[:unlock_token]
+      render :show
+    else
+      attempt_unlock
+    end
+  end
+
+  # GET /resource/unlock/confirm?unlock_token=abcdef
+  def confirm
+    attempt_unlock
+  end
+
+  protected
+
+  # The path used after sending unlock password instructions
+  def after_sending_unlock_instructions_path_for(resource)
+    new_session_path(resource) if is_navigational_format?
+  end
+
+  # The path used after unlocking the resource
+  def after_unlock_path_for(resource)
+    new_session_path(resource) if is_navigational_format?
+  end
+
+  def translation_scope
+    'devise.unlocks'
+  end
+
+  private
+
+  def attempt_unlock
     self.resource = resource_class.unlock_access_by_token(params[:unlock_token])
     yield resource if block_given?
 
@@ -33,20 +65,4 @@ def show
       respond_with_navigational(resource.errors, status: :unprocessable_entity){ render :new }
     end
   end
-
-  protected
-
-    # The path used after sending unlock password instructions
-    def after_sending_unlock_instructions_path_for(resource)
-      new_session_path(resource) if is_navigational_format?
-    end
-
-    # The path used after unlocking the resource
-    def after_unlock_path_for(resource)
-      new_session_path(resource)  if is_navigational_format?
-    end
-
-    def translation_scope
-      'devise.unlocks'
-    end
 end

app/views/devise/unlocks/show.html.erb

@@ -0,0 +1,7 @@
+<h2>Confirm unlock</h2>
+
+<p>Click the link below to finish unlocking your account</p>
+
+<p><%= link_to "Unlock account", confirm_unlock_path(resource_name, unlock_token: @token) %></p>
+
+<%= render "devise/shared/links" %>

lib/devise.rb

@@ -192,6 +192,10 @@ module Test
   mattr_accessor :unlock_in
   @@unlock_in = 1.hour
 
+  # Defines if an extra step is used to unlock an account
+  mattr_accessor :extra_step
+  @@extra_step = false
+
   # Defines which key will be used when recovering the password for an account
   mattr_accessor :reset_password_keys
   @@reset_password_keys = [:email]

lib/devise/models/lockable.rb

@@ -20,6 +20,7 @@ module Models
     #   * +unlock_strategy+: unlock the user account by :time, :email, :both or :none.
     #   * +unlock_in+: the time you want to unlock the user after lock happens. Only available when unlock_strategy is :time or :both.
     #   * +unlock_keys+: the keys you want to use when locking and unlocking an account
+    #   * +extra_step+: if an extra step is used to unlock an account
     #
     module Lockable
       extend  ActiveSupport::Concern
@@ -207,7 +208,7 @@ def lock_strategy_enabled?(strategy)
           self.lock_strategy == strategy
         end
 
-        Devise::Models.config(self, :maximum_attempts, :lock_strategy, :unlock_strategy, :unlock_in, :unlock_keys, :last_attempt_warning)
+        Devise::Models.config(self, :maximum_attempts, :lock_strategy, :unlock_strategy, :unlock_in, :unlock_keys, :last_attempt_warning, :extra_step)
       end
     end
   end

lib/devise/modules.rb

@@ -22,7 +22,7 @@
   # The ones which can sign out after
   routes = [nil, :new]
   d.add_module :confirmable,  controller: :confirmations, route: { confirmation: routes }
-  d.add_module :lockable,     controller: :unlocks,       route: { unlock: routes }
+  d.add_module :lockable,     controller: :unlocks,       route: { unlock: (routes << :confirm) }
   d.add_module :timeoutable
 
   # Stats for last, so we make sure the user is really signed in

lib/devise/rails/routes.rb

@@ -393,8 +393,15 @@ def devise_confirmation(mapping, controllers) #:nodoc:
 
       def devise_unlock(mapping, controllers) #:nodoc:
         if mapping.to.unlock_strategy_enabled?(:email)
-          resource :unlock, only: [:new, :create, :show],
-            path: mapping.path_names[:unlock], controller: controllers[:unlocks]
+          options = {
+            only: [:new, :create, :show],
+            path: mapping.path_names[:unlock],
+            controller: controllers[:unlocks]
+          }
+
+          resource :unlock, **options do
+            get :confirm
+          end
         end
       end
 

lib/generators/templates/controllers/unlocks_controller.rb

@@ -16,6 +16,11 @@ class <%= @scope_prefix %>UnlocksController < Devise::UnlocksController
   #   super
   # end
 
+  # GET /resource/unlock/confirm?unlock_token=abcdef
+  # def confirm
+  #   super
+  # end
+
   # protected
 
   # The path used after sending unlock password instructions

lib/generators/templates/devise.rb

@@ -216,6 +216,9 @@
   # Warn on the last attempt before the account is locked.
   # config.last_attempt_warning = true
 
+  # Defines if an extra step is used to unlock an account
+  # config.extra_step = false
+
   # ==> Configuration for :recoverable
   #
   # Defines which key will be used when recovering the password for an account

lib/generators/templates/simple_form_for/unlocks/show.html.erb

@@ -0,0 +1,7 @@
+<h2>Confirm unlock</h2>
+
+<p>Click the link below to finish unlocking your account</p>
+
+<p><%= link_to "Unlock account", confirm_unlock_path(resource_name, unlock_token: @token) %></p>
+
+<%= render "devise/shared/links" %>

test/generators/views_generator_test.rb

@@ -41,7 +41,6 @@ class ViewsGeneratorTest < Rails::Generators::TestCase
     assert_files nil, mail_template_engine: "markerb"
   end
 
-
   test "Assert only views within specified directories" do
     run_generator %w(-v sessions registrations)
     assert_file "app/views/devise/sessions/new.html.erb"
@@ -68,7 +67,7 @@ class ViewsGeneratorTest < Rails::Generators::TestCase
     run_generator %w(-v registrations -b simple_form_for)
     assert_file "app/views/devise/registrations/new.html.erb", /simple_form_for/
     assert_no_file "app/views/devise/confirmations/new.html.erb"
-    end
+  end
 
   test "Assert specified directories with markerb" do
     run_generator %w(--markerb -v passwords mailer)
@@ -93,6 +92,7 @@ def assert_files(scope = nil, options = {})
     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"
+    assert_file "app/views/#{scope}/unlocks/show.html.erb"
   end
 
   def assert_shared_links(scope = nil)
@@ -105,6 +105,7 @@ def assert_shared_links(scope = nil)
     assert_file "app/views/#{scope}/registrations/new.html.erb", link
     assert_file "app/views/#{scope}/sessions/new.html.erb", link
     assert_file "app/views/#{scope}/unlocks/new.html.erb", link
+    assert_file "app/views/#{scope}/unlocks/show.html.erb", link
   end
 
   def assert_error_messages(scope = nil)

test/integration/lockable_test.rb

@@ -130,6 +130,29 @@ def send_unlock_request
     end
   end
 
+  test 'visiting unlock link should not unlock account when extra_step is true' do
+    user = create_user
+    raw = user.lock_access!
+    swap Devise, extra_step: true do
+      visit_user_unlock_with_token(raw)
+    end
+
+    assert_current_url "/users/unlock?unlock_token=#{raw}"
+    assert_template 'unlocks/show'
+    assert user.reload.access_locked?
+  end
+
+  test 'visiting confirm unlock link should unlock account when extra_step is true' do
+    user = create_user
+    raw = user.lock_access!
+    swap Devise, extra_step: true do
+      visit confirm_user_unlock_path(unlock_token: raw)
+    end
+
+    assert_current_url "/users/sign_in"
+    assert_not user.reload.access_locked?
+  end
+
   test 'user should be able to request a new unlock token via JSON request and should return empty and valid response' do
     user = create_user(locked: true)
     ActionMailer::Base.deliveries.clear
@@ -208,7 +231,6 @@ def send_unlock_request
       assert_current_url "/users/sign_in"
 
       assert_contain "If your account exists, you will receive an email with instructions for how to unlock it in a few minutes."
-
     end
   end
 
@@ -229,5 +251,4 @@ def send_unlock_request
       assert_not_contain "locked"
     end
   end
-
 end

test/models_test.rb

@@ -84,6 +84,10 @@ def assert_include_modules(klass, *modules)
     assert_equal 10.days, Configurable.unlock_in
   end
 
+  test 'set a default value for extra_step' do
+    assert_not Configurable.extra_step
+  end
+
   test 'set null fields on migrations' do
     # Ignore email sending since no email exists.
     klass = Class.new(Admin) do

test/routes_test.rb

@@ -67,6 +67,11 @@ class DefaultRoutingTest < ActionController::TestCase
     assert_recognizes({controller: 'devise/unlocks', action: 'show'}, {path: 'users/unlock', method: :get})
   end
 
+  test 'map confirm user unlock' do
+    assert_recognizes({controller: 'devise/unlocks', action: 'confirm'}, {path: 'users/unlock/confirm', method: :get})
+    assert_named_route "/users/unlock/confirm", :confirm_user_unlock_path
+  end
+
   test 'map new user registration' do
     assert_recognizes({controller: 'devise/registrations', action: 'new'}, 'users/sign_up')
     assert_named_route "/users/sign_up", :new_user_registration_path