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

Closed
rnewman57 opened this Issue Feb 21, 2010 · 5 comments

Comments

Projects
None yet
4 participants

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.

Owner

croaky commented Feb 21, 2010

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).

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 Feb 25, 2010

@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.

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.

Owner

croaky commented Feb 13, 2011

This should now be fixed with this commit:

1c4e1a6

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.

@qrush qrush pushed a commit to qrush/clearance that referenced this issue May 24, 2012

@croaky croaky [#74] initialize salt for old user records without salt 8eca763

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment