Extend remember deadline also while logged in

[janko]

Follow-up to https://github.com/jeremyevans/rodauth/discussions/311

When extending remember deadline is turned on, this makes the user remembered for N days after their last activity, as opposed to since the last time they were autologged in via remember token. This behavior is probably closer to what the end user expects when asking the app to remember them.

To avoid executing an update on every request, we wait for a period of time before extending the remember deadline, 1 hour by default.

[janko]

While implementing this, I found it a bit challenging to understand everything #load_memory method is doing, to know where best to hook extending remember deadline. So, I extracted retrieving the account from remember cookie into a separate method, hopefully this makes the logic a bit clearer.

[jeremyevans]

I merged this and then refactored a bit. Can you check a5c381e5ac26260b4a5c894a4aa4036741d2c0a9 and see if it makes sense to you?

[janko]

Thanks for merging and refactoring, I like the design improvements you've made 👌🏻 The only thing I don't find ideal is not extending the deadline while logged via something other than a remember cookie, because it wouldn't provide the user experience I appreciate about GitHub.

The way I use my computer is that I turn it off once every 2 months or so, mainly to install updates, and don't close my browser in between. If GitHub didn't keep extending the remember deadline after I logged in with a password, my remember cookie would have long expired, and I would have to log back in after turning on my computer, even though I had used GitHub just before restarting.

I guess as long as I'm remembered once, the deadline will continue extending, but logging in with a password breaks that cycle. So, in certain circumstances I'm not remembered within N days of my last activity anymore. I'm not sure what are the security issues around keeping a remember token, but for me there is a lot of value in the UX it would provide.

[jeremyevans]

I would expect most apps using the remember feature are going to have rodauth.load_memory as one of the first things in the routing tree (even before r.rodauth), so a memory would always be used for automatic login if it exists. So this should only affect apps where that isn't the case (where the memory isn't loaded automatically in all cases). The issue with automatically extending memory tokens when not in use is the same issue with automatically extending any token not in use, in that an attacker who gets access to an old token may be able to use it because the old token was continually extended. One example is when you forgot you set a remember token, and always login via a password, but someone gets access to your laptop and is able to find a way to get the remember token to load so they don't need to login. If you aren't actively using the remember token, it's better to let it expire, at least by default, IMO. Maybe I'm being too paranoid, because the remember feature in general shouldn't be used for anything where security is critical (as then you would always want users to authenticate).

That being said, I value your opinion, so if you think this is important, we can add a configuration method so you can get the behavior you want.

Related to this, I think it may be a better idea to allow for multiple remember keys per account, so that each remember extension would remove the existing remember key and create a new remember key, and different browsers would have different remember keys. That would be a major change, so if we were going that route, it would probably be through a new feature (which may depend on the remember feature, or not, depending on implementation).

[janko]

To be clear, I was talking only about applications that call rodauth.load_memory at the top of the routing tree. For me, the problem with the unused remember token is circular; after the initial password login when remember_login is called, if we wait with extending the remember deadline until the remember token has been used for the first time, the token might expire before it could ever get used, even if the user uses the app frequently (which is my case).

In your example, if the user would keep logging in via a password, then either their remember token is expiring or they're logging out. In both cases, the remember token would only be renewed if the user chooses it on login (unless the app is setup to automatically remember logged in users). If their remember token would keep getting extended, then they would stay logged in even after closing the browser or restarting their computer, which should hopefully remind them that they're being remembered.

So, I'm not seeing how always extending remember deadline is less safe than starting it after first autologin via remember cookie. For me the main difference between remember token and other types of tokens (e.g. reset password, email auth) is that they cannot be retrieved from an email, they're not usable after logout (since the cookie is deleted), and they're tied to physical computers (extending the remember cookie on computer A doesn't extend it on computer B).

It would be great to have a way in Rodauth to extend the remember deadline regardless of authentication method. I was hoping for it to be the default behavior when extend_remember_deadline? is set to true, so that the developer doesn't have to know too much about this feature to get the behavior they probably expect. But of course I would be OK with adding a configuration method to enable this as well, thanks for being open to that 🙂

[janko]

Forgot that the remember token can be grabbed from the browser inspector, and stored somewhere else to be used later from a different computer. For some reason I thought that setting HttpOnly prevents any kind of retrieval, but that's just from JS code.

[jeremyevans]

To be clear, I was talking only about applications that call rodauth.load_memory at the top of the routing tree. For me, the problem with the unused remember token is circular; after the initial password login when remember_login is called, if we wait with extending the remember deadline until the remember token has been used for the first time, the token might expire before it could ever get used, even if the user uses the app frequently (which is my case).

I think this is fixable by making it so if there is already a session key for the remember extension, we keep extending it, and setting the session value when initially remembering. Do you think this patch will work (passes existing specs)?

diff --git a/lib/rodauth/features/remember.rb b/lib/rodauth/features/remember.rb
index f10d0d1..5fba6cb 100644
--- a/lib/rodauth/features/remember.rb
+++ b/lib/rodauth/features/remember.rb
@@ -70,6 +70,7 @@ module Rodauth
             # :nocov:
             when remember_remember_param_value
               remember_login
+              set_extend_remember_deadline_session_value if extend_remember_deadline?
             when remember_forget_param_value
               forget_login
             when remember_disable_param_value
@@ -179,11 +180,11 @@ module Rodauth
     private

     def extend_remember_deadline_while_logged_in?
-      return false unless extend_remember_deadline? && logged_in_via_remember_key?
+      return false unless extend_remember_deadline?

       if extended_at = session[remember_deadline_extended_session_key]
         extended_at + extend_remember_deadline_period < Time.now.to_i
-      else
+      elsif logged_in_via_remember_key?
         # Handle existing sessions before the change to extend remember deadline
         # while logged in.
         true
@@ -193,6 +194,10 @@ module Rodauth
     def extend_remember_deadline
       active_remember_key_ds.update(remember_deadline_column=>Sequel.date_add(Sequel::CURRENT_TIMESTAMP, remember_period))
       remember_login
+      set_extend_remember_deadline_session_value
+    end
+
+    def set_extend_remember_deadline_session_value
       set_session_value(remember_deadline_extended_session_key, Time.now.to_i)
     end

So, I'm not seeing how always extending remember deadline is less safe then starting it after first autologin via remember cookie. For me the main difference between remember token and other types of tokens (e.g. reset password, email auth) is that they cannot be retrieved from an email, they're not usable after logout (since the cookie is deleted), and they're tied to physical computers (extending the remember cookie on computer A doesn't extend it on computer B).

The cookie expiration time isn't extended on computer B. However, since the cookie value remains the same, the cookie can be manually modified and the remember token in it will still be valid for use as long as computer A keeps extending the deadline. This would be fixed by having all computers use different remember key values.

It would be great to have a way in Rodauth to extend the remember deadline regardless of authentication method. I was hoping for it to be the default behavior when extend_remember_deadline? is set to true, so that the developer doesn't have to know too much about this feature to get the behavior they probably expect. But of course I would be OK with adding a configuration method to enable this as well, thanks for being open to that 🙂

I'm open to such a configuration method. However, if you think the approach I proposed above may work, maybe we don't need a configuration method for it.

[janko]

The proposed approach unfortunately doesn't seem like it would help with my use case, because I'm not exposing the remember routes to the user, I'm calling remember_login directly to remember the user. In that case the extension session key would get set only on first extension.

I realized my original proposal in this PR had a major flaw, which is that it would remember logins for users who potentially never opted in for that. Extending the remember deadline should have been skipped when the remember cookie is absent, at the very least.

I don't think I grok the remember feature as well as I thought, so I would need time to think this through more. At the moment I don't feel ready of imagining multiple remember key values, as a single token is already a handful for me 😄

[jeremyevans]

It's possible to move the related code into remember_login, so that applications that call rodauth.remember_login without user involvement are covered.

diff --git a/lib/rodauth/features/remember.rb b/lib/rodauth/features/remember.rb
index f10d0d1..5636f40 100644
--- a/lib/rodauth/features/remember.rb
+++ b/lib/rodauth/features/remember.rb
@@ -134,6 +134,7 @@ module Rodauth
       opts[:httponly] = true unless opts.key?(:httponly) || opts.key?(:http_only)
       opts[:secure] = true unless opts.key?(:secure) || !request.ssl?
       ::Rack::Utils.set_cookie_header!(response.headers, remember_cookie_key, opts)
+      set_session_value(remember_deadline_extended_session_key, Time.now.to_i) if extend_remember_deadline?
     end

     def forget_login
@@ -179,11 +180,11 @@ module Rodauth
     private

     def extend_remember_deadline_while_logged_in?
-      return false unless extend_remember_deadline? && logged_in_via_remember_key?
+      return false unless extend_remember_deadline?

       if extended_at = session[remember_deadline_extended_session_key]
         extended_at + extend_remember_deadline_period < Time.now.to_i
-      else
+      elsif logged_in_via_remember_key?
         # Handle existing sessions before the change to extend remember deadline
         # while logged in.
         true
@@ -193,7 +194,6 @@ module Rodauth
     def extend_remember_deadline
       active_remember_key_ds.update(remember_deadline_column=>Sequel.date_add(Sequel::CURRENT_TIMESTAMP, remember_period))
       remember_login
-      set_session_value(remember_deadline_extended_session_key, Time.now.to_i)
     end

     def account_from_remember_cookie

Only issue here is calling extend_remember_deadline manually doesn't extend the cookie if extend_remember_deadline? is false, but it's probably a bad idea to call extend_remember_deadline if extend_remember_deadline? is false.

[janko]

This behavior looks like it's exactly what I wanted 🤘🏻 If remember_login is called in an after_login hook (either automatically or when a checkbox is checked), that will automatically set the deadline extension session key. When the user continues to browse the site while logged in, the deadline will get periodically extended. When the session expires and the user is remembered from a cookie, the deadline will immediately get extended. If remember_login was never called for the user, the deadline extension session key is never set, and thus extend_remember_deadline never gets called. Sounds to me like all cases are covered 👌🏻

From what I can see, calling extend_remember_deadline should extend remember deadline even when extend_remember_deadline? is false 🤔 Only the deadline extension session key won't get set, but that's only used when extend_remember_deadline? is true.

[jeremyevans]

This behavior looks like it's exactly what I wanted 🤘🏻 If remember_login is called in an after_login hook (either automatically or when a checkbox is checked), that will automatically set the deadline extension session key. When the user continues to browse the site while logged in, the deadline will get periodically extended. When the session expires and the user is remembered from a cookie, the deadline will immediately get extended. If remember_login was never called for the user, the deadline extension session key is never set, and thus extend_remember_deadline never gets called. Sounds to me like all cases are covered 👌🏻

Great. I'll add a test for extending the deadline after logging in with password and calling remember_login, and then commit it.

From what I can see, calling extend_remember_deadline should extend remember deadline even when extend_remember_deadline? is false 🤔 Only the deadline extension session key won't get set, but that's only used when extend_remember_deadline? is true.

Correct. As mentioned, the cookie deadline is not extended, but the remember key is still extended in the database. It's a bit inconsistent, but I think it's acceptable.