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

[michieldewit]

In commit c929966 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.

[josevalim]

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

[michieldewit]

@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

[josevalim]

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

[michieldewit]

@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

[josevalim]

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

[reedloden]

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

[josevalim]

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

[jjoos]

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

[josevalim]

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

[jjoos]

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

[josevalim]

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

[liamwhite]

Still awaiting a fix in Devise for this.

[ulissesalmeida]

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: #4039