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.

Sign up for GitHub

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

Jump to bottom

Require password for account deletion by default #4816

Open
mnlse opened this issue Mar 24, 2018 · 6 comments · May be fixed by #5029 or #5697
Open

Require password for account deletion by default #4816

mnlse opened this issue Mar 24, 2018 · 6 comments · May be fixed by #5029 or #5697
Labels
PR attached

Comments

@mnlse
Copy link

mnlse commented Mar 24, 2018

Current behavior

Devise requires current password in order to change account, but does not require current password for account deletion.

Expected behavior

Devise should require current password in order to delete account.

Manual solution

def destroy
  if resource.destroy_with_password(params[:user][:current_password])
    flash[:notice] = "Your account has been deleted"
    redirect_to root_path    
  else                       
    flash[:alert] = "Wrong password"
    render :edit, layout: 'application'                                                 
  end  
end
@tegon
Copy link
Member

tegon commented Mar 26, 2018

I'm ok with supporting this feature, but we can't require it by default since it isn't backward compatible.
We can add a setting and then check for it in the destroy method.
@mnlse If you're willing to open a pull request for this, we'd be happy to accept it.

Thanks!

@tegon tegon added the Needs PR label Mar 26, 2018
@iorme1
Copy link
Contributor

iorme1 commented Apr 5, 2018

I'll be working on a PR for this.

@iorme1 iorme1 mentioned this issue Apr 12, 2018
Closed
@iorme1
Copy link
Contributor

iorme1 commented Apr 12, 2018

I finished the feature and submitted a PR.

@lancecarlson
Copy link
Contributor

@tegon @iorme1 This looks good to me. If you want this to default to off, I would make the generated config commented out and assigned to true. If you want this to default to on, then I would leave the config uncommented, but change its value to true.

@hayssac hayssac linked a pull request Feb 21, 2019 that will close this issue
Open
@collimarco
Copy link

I am really interested in this feature. It doesn't make sense that a user cannot change the email, but can destroy the account without password!

Can you merge the PR?

@tegon
Copy link
Member

tegon commented Aug 15, 2019

@collimarco The PR still needs some fixes, that's why I haven't merged yet.

@willianveiga willianveiga linked a pull request Jun 21, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
PR attached
Milestone
No milestone
5 participants
@lancecarlson @collimarco @tegon @mnlse @iorme1