Password attribute is emptied when record is invalid using has_secure_password #1386

Closed
Sephi-Chan opened this Issue May 28, 2011 · 6 comments

Comments

Projects
None yet
5 participants
@Sephi-Chan
Contributor

Sephi-Chan commented May 28, 2011

Hello,

In a model (say User) using has_secure_password, the password and it's confirmation are lost if one of the other fields is invalid.

It's really bad for user experience. For instance on a sign up form : if my username or email is invalid, i have to retype the password and its confirmation whereas they were good.

I would fix that but i don't know how. :(

@dmathieu

This comment has been minimized.

Show comment Hide comment
@dmathieu

dmathieu May 29, 2011

Contributor

That's a quite common behavior actually on most web applications.

Moreover I see two problems with this :

  • The unencrypted password is not stored at all. So we can't display it back in the password input field anyway.
  • Displaying it back would mean adding it in the HTML code. So anyone whom could view the page's source would see the password. It could be a security issue for some.

-1 on this.

Contributor

dmathieu commented May 29, 2011

That's a quite common behavior actually on most web applications.

Moreover I see two problems with this :

  • The unencrypted password is not stored at all. So we can't display it back in the password input field anyway.
  • Displaying it back would mean adding it in the HTML code. So anyone whom could view the page's source would see the password. It could be a security issue for some.

-1 on this.

@Sephi-Chan

This comment has been minimized.

Show comment Hide comment
@Sephi-Chan

Sephi-Chan May 29, 2011

Contributor

Devise don't lose the password, authlogic neither.
Major websites (Facebook, Twitter, Digg, etc.) don't lose the password.

There is no more security flaw the second time you type than the first. In both case the password is somewhere in the browser (and accessible through Javascript, then with both Javascript and HTML). What kind of attack do you have in mind ?

The main flaw is about user experience : i mistype my email so i have to retype the password and its confirmation ? Not good at all. :(

Ps : Big up Oahu ! :)

Contributor

Sephi-Chan commented May 29, 2011

Devise don't lose the password, authlogic neither.
Major websites (Facebook, Twitter, Digg, etc.) don't lose the password.

There is no more security flaw the second time you type than the first. In both case the password is somewhere in the browser (and accessible through Javascript, then with both Javascript and HTML). What kind of attack do you have in mind ?

The main flaw is about user experience : i mistype my email so i have to retype the password and its confirmation ? Not good at all. :(

Ps : Big up Oahu ! :)

@farleyknight

This comment has been minimized.

Show comment Hide comment
@farleyknight

farleyknight May 29, 2011

Contributor

If you want to check that the password and it's confirmation match, and you're worried about user experience, you should go with a Javascript solution, like the jQuery validation plugin[1]. Then your user will know instantly. Similarly, if you're checking the password for strength, the same plugin allows for custom validation methods.

[1] http://docs.jquery.com/Plugins/validation

Contributor

farleyknight commented May 29, 2011

If you want to check that the password and it's confirmation match, and you're worried about user experience, you should go with a Javascript solution, like the jQuery validation plugin[1]. Then your user will know instantly. Similarly, if you're checking the password for strength, the same plugin allows for custom validation methods.

[1] http://docs.jquery.com/Plugins/validation

@rawsyntax

This comment has been minimized.

Show comment Hide comment
@rawsyntax

rawsyntax May 29, 2011

part of the problem appears to be that password_field has {:value => nil} merged in as part of the helper (see actionpack/lib/action_view/helpers/form_helper.rb )

The other part is that password_confirmation attribute is virtual (in memory only).

part of the problem appears to be that password_field has {:value => nil} merged in as part of the helper (see actionpack/lib/action_view/helpers/form_helper.rb )

The other part is that password_confirmation attribute is virtual (in memory only).

@sikachu

This comment has been minimized.

Show comment Hide comment
@sikachu

sikachu Jun 1, 2011

Member

I think this is normal, no matter you're using has_secure_password or not. Your application/browser should reset your password field if there's any validation error. IMO I never see an application that would retain user's password in case the validation of the model fails.

If you want to make sure use does correctly type in the same password for confirmation, you should use client-side validation as suggested by @farleyknight.

Member

sikachu commented Jun 1, 2011

I think this is normal, no matter you're using has_secure_password or not. Your application/browser should reset your password field if there's any validation error. IMO I never see an application that would retain user's password in case the validation of the model fails.

If you want to make sure use does correctly type in the same password for confirmation, you should use client-side validation as suggested by @farleyknight.

@sikachu sikachu closed this Jun 1, 2011

@Sephi-Chan

This comment has been minimized.

Show comment Hide comment
@Sephi-Chan

Sephi-Chan Jun 1, 2011

Contributor

Such a bad faith is quite disappointing. For information: GitHub and Twitter do retain the user's password when the registration fail at validation.

Anyway i will go for client-side validation.

Contributor

Sephi-Chan commented Jun 1, 2011

Such a bad faith is quite disappointing. For information: GitHub and Twitter do retain the user's password when the registration fail at validation.

Anyway i will go for client-side validation.

jake3030 pushed a commit to jake3030/rails that referenced this issue Jun 28, 2011

Let polymorphic_path treat an array contains single name as without a…
…rray [#1386 state:committed]

Signed-off-by: David Heinemeier Hansson <david@loudthinking.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment