Add additional step for unlocking account

TomGibbon · TomGibbon · commit c09e21721c65 · 2025-08-11T09:32:28.000+01:00
diff --git a/app/controllers/devise/unlocks_controller.rb b/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
diff --git a/app/views/devise/unlocks/show.html.erb b/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" %>
diff --git a/lib/devise.rb b/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]
diff --git a/lib/devise/models/lockable.rb b/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
diff --git a/lib/devise/modules.rb b/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
diff --git a/lib/devise/rails/routes.rb b/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
 
diff --git a/lib/generators/templates/controllers/unlocks_controller.rb b/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
diff --git a/lib/generators/templates/devise.rb b/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
diff --git a/lib/generators/templates/simple_form_for/unlocks/show.html.erb b/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" %>
diff --git a/test/generators/views_generator_test.rb b/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)
diff --git a/test/integration/lockable_test.rb b/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
diff --git a/test/models_test.rb b/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
diff --git a/test/routes_test.rb b/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

Original file line number	Diff line number	Diff line change
`@@ -216,6 +216,9 @@`
`216`	`216`	`# Warn on the last attempt before the account is locked.`
`217`	`217`	`# config.last_attempt_warning = true`
`218`	`218`
	`219`	`+ # Defines if an extra step is used to unlock an account`
	`220`	`+ # config.extra_step = false`
	`221`	`+`
`219`	`222`	`# ==> Configuration for :recoverable`
`220`	`223`	`#`
`221`	`224`	`# Defines which key will be used when recovering the password for an account`