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

Update Password Succeeds Without Password Confirmation #2942

Closed
joelcdoyle opened this issue Mar 24, 2014 · 14 comments
Closed

Update Password Succeeds Without Password Confirmation #2942

joelcdoyle opened this issue Mar 24, 2014 · 14 comments

Comments

@joelcdoyle
Copy link

I have created an API using devise. When creating a user, it is required that a password and a password_confirmation be provided, and that they match. However, I can't replicate this behavior when updating a user's password.

When updating a user's password, I would like a valid current_password, password, and password_confirmation to be provided

I'm whitelisting :current_password, :password, and :password_confirmation with strong params in my controller, and using update_with_password to update the password attribute on my user instance.

I have a gist here:
https://gist.github.com/joelcdoyle/9747192

@josevalim
Copy link
Contributor

You need to give us more information on how to reproduce this issue, otherwise there is nothing we can do. Please read CONTRIBUTING.md file for more information about creating bug reports. Thanks!

@josevalim josevalim reopened this Mar 24, 2014
@josevalim
Copy link
Contributor

Reopening due to #1947.

@joelcdoyle
Copy link
Author

I added my Gemfile.lock to the gist

@josevalim
Copy link
Contributor

I still can't reproduce the bug though. Which data are you sending to the form when it fails? I mean, which fields are being filled with which value?

@joelcdoyle
Copy link
Author

I'm calling update_with_password on the user instance with :current_password set to the users's current password, :password set to a new valid password, and :password_confirmation is nil

It's an API so there are no forms. All parameters are coming in from a JSON request body

See line #20. A PUT to users#update with password, and current_password params, and no password_confirmation

    #this test fails. Devise allows the password to change without providing confirmation
    it "should reject request if password confirmation is not provided" do
      user = User.create!(email: "bob@example.com", password: "validPassword", password_confirmation: "validPassword")
      sign_in user
      put :update, {id: user.id, password: "newPassword", current_password: "validPassword" }
      #actuall response is 200 (OK)
      response.status.should be 422
    end

@josevalim
Copy link
Contributor

That's how password_confirmation is designed to work. If it is nil, it won't check for the confirmation because we don't want to force APIs, tests and so on to pass the confirmation. We only want forms to send the confirmation and you can do it by guaranteeing a blank value is sent (which is the default for html forms). Please check the second part of this post: http://blog.plataformatec.com.br/2009/08/do-not-burden-your-users-with-validations/

@joelcdoyle
Copy link
Author

Yes, but in my case when the user is created, password_confirmation IS NOT NIL. The user is created by requiring a password_confirmation. So, it is indeed not nil.

Further, I am not testing the presence of password_confirmation. I am testing to ensure that the password cannot be changed without requiring three params (:current_password, :password, :password_confirmation)

@joelcdoyle
Copy link
Author

I guess I'm not understanding why you would require a password confirmation on creation, but not on update or modification?

@josevalim
Copy link
Contributor

If you write a test like this:

post :create, {id: user.id, password: "newPassword", email: "..." }

The password_confirmation won't be required by default too... i.e. it is not related to the resource status it is simply related to the fact password_confirmation is nil.

@joelcdoyle
Copy link
Author

The comment above this method is misleading (to me, at least) by saying that the params are rejected:
Update record attributes when :current_password matches, otherwise returns error on :current_password. It also automatically rejects :password and :password_confirmation if they are blank.

"automatically rejects" to me, sounds like "returns false" or "returns error"

@josevalim
Copy link
Contributor

Yes, update_with_password docs could be improved but that's not the issue here. The "issue" is just how confirmations' validation works on Rails: if the confirmation is nil, the confirmation is not validated.

@joelcdoyle
Copy link
Author

Ok, I can accept that. That does seem to be correct.

@josevalim
Copy link
Contributor

Thanks!

@TomasBarry
Copy link

Hi @josevalim,

Is there a way other than using validates_presence_of :password_confirmation, :if => :password_changed? to force Devise to require a password confirmation on all updates and to fail if the password_confirmation is nil?

karunkumar1ly pushed a commit to edcast/devise that referenced this issue Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants