Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature require_password_to_destroy #5697

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions app/controllers/devise/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ def update
end

# DELETE /resource
def destroy
resource.destroy
Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name)
set_flash_message! :notice, :destroyed
yield resource if block_given?
respond_with_navigational(resource){ redirect_to after_sign_out_path_for(resource_name), status: Devise.responder.redirect_status }
def destroy(&block)
if destroy_resource(resource, account_destroy_params)
sign_out_resource(resource_name, resource, &block)
else
render :edit, status: 422
end
end

# GET /resource/cancel
Expand All @@ -94,6 +94,14 @@ def update_resource(resource, params)
resource.update_with_password(params)
end

def destroy_resource(resource, params)
if resource.class.require_password_to_destroy
resource.destroy_with_password(params)
else
resource.destroy
end
end

# Build a devise resource passing in the session. Useful to move
# temporary session data to the newly created user.
def build_resource(hash = {})
Expand Down Expand Up @@ -141,6 +149,10 @@ def account_update_params
devise_parameter_sanitizer.sanitize(:account_update)
end

def account_destroy_params
devise_parameter_sanitizer.sanitize(:account_destroy)[:current_password]
end

def translation_scope
'devise.registrations'
end
Expand All @@ -160,6 +172,13 @@ def set_flash_message_for_update(resource, prev_unconfirmed_email)
set_flash_message :notice, flash_key
end

def sign_out_resource(resource_name, resource, &block)
Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name)
set_flash_message! :notice, :destroyed
yield resource if block_given?
respond_with_navigational(resource){ redirect_to after_sign_out_path_for(resource_name), status: Devise.responder.redirect_status }
end

def sign_in_after_change_password?
return true if account_update_params[:password].blank?

Expand Down
20 changes: 18 additions & 2 deletions app/views/devise/registrations/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<h2>Edit <%= resource_name.to_s.humanize %></h2>

<%= form_for(resource, as: resource_name, url: registration_path(resource_name), html: { method: :put }) do |f| %>
<%= render "devise/shared/error_messages", resource: resource %>
<%= render "devise/shared/error_messages", resource: resource if params[:action] == "update" %>

<div class="field">
<p><%= f.label :email %></p>
Expand Down Expand Up @@ -37,6 +37,22 @@

<h3>Cancel my account</h3>

<div>Unhappy? <%= button_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?", turbo_confirm: "Are you sure?" }, method: :delete %></div>
<% if resource.class.require_password_to_destroy %>
<%= form_for(resource, as: resource_name, url: registration_path(resource_name), html: { name: 'require_password_to_destroy', method: :delete }) do |f| %>
<%= render "devise/shared/error_messages", resource: resource if params[:action] == "destroy" %>

<div class="field destroy-password">
<%= f.label :current_password, for: :require_password_to_destroy_current_password %>
<i>(we need your current password to delete your account)</i><br />
<%= f.password_field :current_password, id: :require_password_to_destroy_current_password, autocomplete: "off" %>
</div>

<div class="actions">
<%= f.submit "Cancel my account" %>
</div>
<% end %>
<% else %>
<div>Unhappy? <%= button_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?", turbo_confirm: "Are you sure?" }, method: :delete %></div>
<% end %>

<%= link_to "Back", :back %>
4 changes: 4 additions & 0 deletions lib/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,10 @@ module Test
mattr_accessor :sign_in_after_change_password
@@sign_in_after_change_password = true

# When true, signed in users must provide their current password to cancel account
mattr_accessor :require_password_to_destroy
@@require_password_to_destroy = false

# Default way to set up Devise. Run rails generate devise_install to create
# a fresh initializer with all configuration values.
def self.setup
Expand Down
1 change: 1 addition & 0 deletions lib/devise/models/registerable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def new_with_session(params, session)
end

Devise::Models.config(self, :sign_in_after_change_password)
Devise::Models.config(self, :require_password_to_destroy)
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/devise/parameter_sanitizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ class ParameterSanitizer
DEFAULT_PERMITTED_ATTRIBUTES = {
sign_in: [:password, :remember_me],
sign_up: [:password, :password_confirmation],
account_update: [:password, :password_confirmation, :current_password]
account_update: [:password, :password_confirmation, :current_password],
account_destroy: [:current_password]
}

def initialize(resource_class, resource_name, params)
Expand Down
3 changes: 3 additions & 0 deletions lib/generators/templates/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@
# Defines which key will be used when confirming an account
# config.confirmation_keys = [:email]

# If true, requires user to provide password to delete record.
config.require_password_to_destroy = false

# ==> Configuration for :rememberable
# The time the user will be remembered without asking for credentials again.
# config.remember_for = 2.weeks
Expand Down
14 changes: 14 additions & 0 deletions test/controllers/custom_registrations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,20 @@ class CustomRegistrationsControllerTest < Devise::ControllerTestCase
assert @controller.update_block_called?, "update failed to yield resource to provided block"
end

test "yield resource to block on destroy failure" do
sign_in @user
delete :destroy, params: { user: { } }
assert @controller.destroy_block_called?, "destroy failed to yield resource to provided block"
end

test "yield resource to block on destroy success" do
swap Devise, require_password_to_destroy: true do
sign_in @user
delete :destroy, params: { user: { current_password: @password } }
assert @controller.destroy_block_called?, "destroy failed to yield resource to provided block"
end
end

test "yield resource to block on new" do
get :new
assert @controller.new_block_called?, "new failed to yield resource to provided block"
Expand Down
3 changes: 1 addition & 2 deletions test/generators/views_generator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ def assert_shared_links(scope = nil)

def assert_error_messages(scope = nil)
scope = "devise" if scope.nil?
link = /<%= render \"#{scope}\/shared\/error_messages\", resource: resource %>/

link = /<%= render \"#{scope}\/shared\/error_messages\", resource: resource( if params\[:action\] == "(update|destroy)")? %>/
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
Expand Down
33 changes: 33 additions & 0 deletions test/integration/registerable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,39 @@ def user_sign_up
assert_empty User.to_adapter.find_all
end

test 'a signed in user should be able to cancel their account with current password if password required' do
swap Devise, require_password_to_destroy: true do
sign_in_as_user
get edit_user_registration_path

within(".destroy-password") do
fill_in 'current password', with: '12345678'
end

click_button "Cancel my account"
assert_contain "Bye! Your account has been successfully cancelled. We hope to see you again soon."
end
assert User.to_adapter.find_all.empty?
end

test 'a signed in user should not be able to cancel their account with incorrect password if password required' do
swap Devise, require_password_to_destroy: true do
sign_in_as_user
get edit_user_registration_path

within(".destroy-password") do
fill_in 'current password', with: '87654321'
end

click_button "Cancel my account"
assert_contain "Current password is invalid"

assert_current_url '/users'

refute User.to_adapter.find_all.empty?
end
end

test 'a user should be able to cancel sign up by deleting data in the session' do
get "/set"
assert_equal "something", @request.session["devise.foo_bar"]
Expand Down
10 changes: 10 additions & 0 deletions test/rails_app/app/controllers/custom/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ def update
end
end

def destroy
super do |resource|
@destroy_block_called = true
end
end

def create_block_called?
@create_block_called == true
end
Expand All @@ -30,4 +36,8 @@ def update_block_called?
def new_block_called?
@new_block_called == true
end

def destroy_block_called?
@destroy_block_called == true
end
end