diff --git a/app/controllers/devise/unlocks_controller.rb b/app/controllers/devise/unlocks_controller.rb index b1487760b6..d9c93e5471 100644 --- 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 new file mode 100644 index 0000000000..cf76dc6960 --- /dev/null +++ b/app/views/devise/unlocks/show.html.erb @@ -0,0 +1,7 @@ +
Click the link below to finish unlocking your account
+ +<%= link_to "Unlock account", confirm_unlock_path(resource_name, unlock_token: @token) %>
+ +<%= render "devise/shared/links" %> diff --git a/lib/devise.rb b/lib/devise.rb index 5b7417ed6c..b19a47f45e 100644 --- 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 index 65bb400d0e..5575c3520c 100644 --- 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 index d8cde834c1..44d21eec18 100644 --- 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 index f43e62fea7..64480968e6 100644 --- 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 index 0eadbbf65b..2e9de38f30 100644 --- 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 index 9e6744bd7d..5745cfd52c 100644 --- 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 new file mode 100644 index 0000000000..cf76dc6960 --- /dev/null +++ b/lib/generators/templates/simple_form_for/unlocks/show.html.erb @@ -0,0 +1,7 @@ +Click the link below to finish unlocking your account
+ +<%= link_to "Unlock account", confirm_unlock_path(resource_name, unlock_token: @token) %>
+ +<%= render "devise/shared/links" %> diff --git a/test/generators/views_generator_test.rb b/test/generators/views_generator_test.rb index 1f8f90f3ca..e1292fb5d0 100644 --- 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 index e5dd5ee08b..6176ca7333 100644 --- 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 index 16acb92c98..fc306add80 100644 --- 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 index 20ba311727..7c1578a429 100644 --- 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