thoughtbot / clearance

Rails authentication with email & password.
https://thoughtbot.com
MIT License
3.71k stars 467 forks source link

User salt and remember_token fields remain nil forever after ClearanceUpdateUsers migration #74

Closed rnewman57 closed 13 years ago

rnewman57 commented 14 years ago

If some User records already exist when the ClearanceUpdateUsers migration runs, their salt and remember_token fields are initialized to nil. They will never become non-nil, no matter how many times these users subsequently attempt to login, confirm their accounts, or change their passwords.

When they attempt to login, the login attempt intially appears successful, but actually fails (with no error message) because the remember_token is still nil and is stored in the cookie.

croaky commented 14 years ago

Interesting side effect. Why didn't the salt and remember_token columns exist in your application? That migration only creates columns that didn't previously exist.

I started to make this change:

http://github.com/thoughtbot/clearance/commit/8eca763745dc845c90808107addfbb6d64e8f219

Then rolled it back. I think the better solution would be to write and run a rake task that goes through your Users and just calls user.send(:initialize_salt); user.send(:encrypt_password).

rnewman57 commented 14 years ago

My application originally had a users table but no authentication at all, and then I added Clearance to it. So Clearance gave me a migration that added these two columns (and others), but did not initialize them to non-nil values in my already existing user records.

user.send(:initialize_salt) will not work because new_record? will always return false. Your proposed change above would fix this, so I think it's a good idea.

andhapp commented 14 years ago

@dan - why did you abandon your work? This seems like a possible use-case. A user can move to Clearance mid-way in the project and this would create a problem. I am not sure about the best way to approach this problem but it is worth pondering upon.

KingOfBrian commented 14 years ago

I just got bit with this myself upgrading from 0.7.0. The remember_token field was null so none of my users could submit.

User.find(:all).each {|u| u.reset_remember_token!}

Fixed things. Seems like that should be added to the migration. Wasn't rocket science but quite a pain to track down.

croaky commented 13 years ago

This should now be fixed with this commit:

https://github.com/thoughtbot/clearance/commit/1c4e1a6b457e836ef52ca532e729ab118fff083e

The way it works now, your old users just need to go through the "Forgot password" flow to sign in.

There's nothing you as the developer should do in terms of creating salts, remember tokens, etc. because the user will have to create a password at some point. This way, it's hands-off for you. You just install Clearance and it handles this case.