Devise: Remember_token always regenerated when user logs in: existing one never kept anymore

Created on 11 Feb 2016  路  20Comments  路  Source: heartcombo/devise

In commit https://github.com/plataformatec/devise/commit/c92996646aba2d25b2c3e235fe0c4f1a84b70d24 a change was made that causes remember_tokens to be regenerated without checking whether the old one has expired. This causes a problem when users log in on multiple browsers or devices. Every time they log in, their session on other browsers/devices is invalidated, because a new token is generated.

My project is forced to stick with Devise 3.5.3 because of this breaking change. The fact that the commit I mentioned still contains a TODO is probably indicative of the fact this is a problem to be resolved.

Bug

Most helpful comment

@jjoos because you can use the remember token completely without tokens, by relying on the authentication salt

All 20 comments

@michieldewit are you using the manual remember_token? If so, I believe you are right. I completely forgot about this case, sorry about this. We need to tackle it.

@josevalim We use stored remember_tokens indeed. Going back to 3.5.3 did not work for us either, btw, because of the new format of the remember cookies. Right now, we monkey patched Devise::Models::Rememberable.remember_me! in our User model to work more or less like in the old situation. It's ugly, but it works:

def remember_me!
    token_expired = self.remember_token.blank? || self.remember_created_at.blank? || self.remember_created_at + self.class.remember_for < Time.now.utc
    if token_expired
      self.remember_token = self.class.remember_token
      self.remember_created_at = Time.now.utc
      save(validate: false) if self.changed?
    end
  end

@michieldewit you no longer need to renew or expire the token. So my suggestion would be to just set it if none is set. If you want to expire existing tokens, you just clean the database column for the desired rows.

@josevalim About the expiration you are probably right. Still it feels a little safer to know that remembered sign ins will automatically expire when a user has not been active for X days

@michieldewit this property is now intrinsic to the new system, that's why we don't need to worry about it at the token level. :)

Any update on this? We've add the monkey patch, but would prefer this fixed in Devise.

Thanks @reedloden and @michieldewit . I think a simpler fix on this is to rewrite the remember_me function to work like this:

      def remember_me!(*)
        self.remember_token ||= self.class.remember_token if respond_to?(:remember_token)
        self.remember_created_at ||= Time.now.utc
        save(validate: false) if self.changed?
      end

Can you please confirm this also works as expected? I only changed the remember_token assignment to use ||=.

I can confirm the simpler fix works, wouldn't this be a bit easier to read though?

  def remember_me!(*)
    return unless respond_to?(:remember_token)

    self.remember_token ||= self.class.remember_token
    self.remember_created_at ||= Time.now.utc
    save(validate: false) if self.changed?
  end

We still need to execute remember_created_at if you don't have respond_to?(:remember_token).

@josevalim Could you explain why we need to execute self.remember_created_at ||= Time.now.utc when self.remember_token isn't being set?

@jjoos because you can use the remember token completely without tokens, by relying on the authentication salt

Still awaiting a fix in Devise for this.

This issue was related to extended remember me configuration was working as always true.

It is fixed on Devise 3.5.7 and 4.0.0.

Feel free to reopen if the problem persist.

Related PR: https://github.com/plataformatec/devise/pull/4039

I just upgraded to Devise 3.5.8 and I'm still seeing this issue; the remember_token is reset every time the user logs in, so that if they log in on browser A, and then later on browser B, the remember_token changes and the session on browser A is invalidated.

Am I supposed to be using extend_remember_period = true to get the behavior I want (i.e. logging in to a second device doesn't log them out on the first). The documentation isn't very verbose about what extend_remember_period does.

@bsoule Thank you for the feedback. The extend_rememeber_period configuration about extends the period of remember token validity. When you set to true, every time the authenticate user use the application with a valid remember token the expiration date of the token will be extended. When it is set to false, the expiration date will never be updated and it will expires.

The option you have suggested is support multiple remember tokens, I think today Devise does not have this option out of the box. I'll check with @josevalim about it. You may want to implement it by yourself or find gem that does.

Feel free to open a pull request to improve the configuration template text or the README documentation with a better explanation about the extend remember me period.

Best

@josevalim What's the expected behaviour when user sign in using a browser with remember me checked with a valid existent remember token from another browser? Should it override the existent one? Is possible today Devise support multiple remember me cookie tokens from different browsers?

Thanks for explaining the extend_remember_period option. Much clearer now :-)

From reading the comments in this issue, it sounds like remember_token should not get overwritten when a user is signing in and a valid token already exists. Above @josevalim proposes a fix where you'd set the remember_token with ||= in remember_me!

If the expected behavior is indeed as described in this thread, then the original issue described still remains in the master branch. I can submit a PR for this.

@bsoule You're right. Feel free to submit a PR with a fix :) . Would be good have a regression test too.

Hi people, if anyone has some time to try out the branch with the fix and give us some feedback. I can release a patch version. The PR with the fix: https://github.com/plataformatec/devise/pull/4101

I think it's fixed now on 3.5.10, 4.0.3 and 4.1.1. :pray:

Was this page helpful?
0 / 5 - 0 ratings