Only one remember token allowed per account

[Bertg]

I was reading through the code, and found this surprising part:

rodauth/lib/rodauth/features/remember.rb

Lines 158 to 167 in 94123d8

	def get_remember_key
	unless @remember_key_value = active_remember_key_ds.get(remember_key_column)
	generate_remember_key_value
	transaction do
	remove_remember_key
	add_remember_key
	end
	end
	nil
	end

The way I'm reading this, is that an account can only have one remember token active at any time. If a user logs into their account in an other browser, the remember token of the other browser gets removed.

Am I reading this correct? Is this a conscious design decision, and what is the rationale behind this?

[jeremyevans]

Remember tokens are shared between browsers, the remove/add is to handle expired keys. If the account already has a remember token, @remember_key_value will be set, and the unless branch will not be taken.

[Bertg]

Or are you objecting to the fact that Rodauth lets you reduce the security by specifying your own options?

Yeah, I don't see why this should be this easy. Rodauth should resist unsafe practices.

You aren't wrong that it lowers security, any application using the feature is making a very deliberate trade-off to decrease security and increase usability.

Well, that is assuming each developer considers the effects of adding such a feature :)

I'm open to backwards-compatible suggestions for improving the security for the remember feature, if you have any.

That is going to be a hassle. Ideal there would be N remember tokens per account. Right now the db structure is using the account id as the primary key for the table.

We could have the remember know about its token strategy. This could be single for the current behaviour, per_device for the suggested behaviour and migrating to allow both strategies at runtime.
Basically the remember feature would then be aware of 2 tables. The current table, and one where the primary key is the token.
In migrating mode, tokens found in the old table would be moved to the new table on the fly.

Release wise you can first release with he single strategy. I guess you would need a major update to switch the default to per_device and an other major update to remove the single strategy.

That sounds like a lot of work.

[jeremyevans]

Or are you objecting to the fact that Rodauth lets you reduce the security by specifying your own options?

Yeah, I don't see why this should be this easy. Rodauth should resist unsafe practices.

I'm against the idea that Rodauth should resist unsafe practices. Rodauth should have safe defaults. If the user wants to change the defaults in a manner that lowers security, I don't think Rodauth should go out of its way to prevent that. Rodauth should not assume it knows better than the user using it.

You should understand that Rodauth allows overriding most of its internals using configuration methods, and if done incorrectly, that can be a source of security issues. For example, let's say Rodauth was strict and didn't allow unsafe cookie options to be used in remember_cookie_options. The user can still override remember_login to use them.

You aren't wrong that it lowers security, any application using the feature is making a very deliberate trade-off to decrease security and increase usability.

Well, that is assuming each developer considers the effects of adding such a feature :)

If developers are adding features without considering the effects of the feature, there isn't much hope for the applications they are developing. :)

I'm open to backwards-compatible suggestions for improving the security for the remember feature, if you have any.

That is going to be a hassle. Ideal there would be N remember tokens per account. Right now the db structure is using the account id as the primary key for the table.

We could have the remember know about its token strategy. This could be single for the current behaviour, per_device for the suggested behaviour and migrating to allow both strategies at runtime. Basically the remember feature would then be aware of 2 tables. The current table, and one where the primary key is the token. In migrating mode, tokens found in the old table would be moved to the new table on the fly.

Release wise you can first release with he single strategy. I guess you would need a major update to switch the default to per_device and an other major update to remove the single strategy.

That sounds like a lot of work.

I think if we wanted to do this, it would be by building a multiple_remember_token feature that builds on top of the existing remeber feature, extending it to support multiple remember tokens per account. This would require a different database schema, as well as code and user interface changes (so users could choose between deleting the current remember token or all remember tokens). Such a feature would support automatic upgrading from the remember token feature by default. We would continue to support both features, and the user could pick which approach they wanted to use.

I'm not against that, though I do think it is a significant amount of work for not much gain. The only advantage is removing a remember token affects only a single browser that has stored that remember token, instead of all browsers that have done the shared remember token.

Is that something you are interested in working on?

[Bertg]

I'm against the idea that Rodauth should resist unsafe practices. Rodauth should have safe defaults. If the user wants to change the defaults in a manner that lowers security, I don't think Rodauth should go out of its way to prevent that. Rodauth should not assume it knows better than the user using it.

Understood.

Is that something you are interested in working on?

Yeah, I'll give it a shot.

[Bertg]

@jeremyevans I'm going to assume you intend this new plugin to be used like this:

enable :login, :remember, :multiple_remember_tokens

[jeremyevans]

The multiple_remember_tokens feature would depend on the remember feature, so you could do: enable :login, :multiple_remember_tokens

[Bertg]

Come to think of it, I think a recent change to active_sessions (6bdd495) may have yielded an unexpected side effect.

When active_sessions is in use, the remember me functionality will not behave as expected in multiple browsers. Effectively, if the user signs out, the remember me token on the other browser will be invalidated.

[jeremyevans]

That's a good point. Maybe we should add an option to not clear the remember token for logouts when using active sessions, for people that would like the previous behavior?

[Bertg]

@jeremyevans I would absolutely add that, as that PR did change the behaviour of the library. I prefer the new change, but other people may not.
What the default should be... I have no clue.

[jeremyevans]

I'm open to such an option, but considering nobody has requested it yet, I'm not sure it's needed. I'll probably wait until someone requests it with the desire to use it.

[Bertg]

@jeremyevans We have come again past this issue, and are working internally towards a solution.

To summarise: The preferred behaviour for us, when using remember and active_sessions combined, would be that the remember cookie gets invalidated when the user session is invalidated (eg, when it is removed from the database).

Now, we want to do this with as little impact as possible, and we thought that the following solution would work, and could maybe be of interest to rodauth.

By default rodauth creates the remember token by combining the account_id and the remember_key_value into the cookie. When used in conjunction with active_sessions however, this could be changed to session_id and remember_key_value.

This affords the same safeties as using the account_id and allows us to effectively destroy the remember token per session, without any changes to the database.

The only hiccup we have found so far, is that the base implementation of the remember_token joins using a _, which is a character that may appear in the session_id which makes managing that one a bit harder.

What do you think? Is this a good solution? Would you be interested in a port of this solution into rodauth?

[jeremyevans]

The point of the remember feature is to allow for automatic login after browser session expires. Generally, the remember token would be valid for significantly longer than the session would be valid for. It doesn't make sense to me to tie the remember token to the active session. So I don't think I would accept a feature that implements the approach.

I'm open to a remember_token_joiner method so you can replace the _ with a different character, to make it easier to implement an external feature that wants to do that.

[Bertg]

Generally, the remember token would be valid for significantly longer than the session would be valid for. It doesn't make sense to me to tie the remember token to the active session.

Yeah, I had not taken that into account. I guess we will have to do it the hard way, and use a new table to make this work.

[jeremyevans]

I would be open to a new feature that was similar to remember but allowed for multiple remember tokens per account (which could potentially depend on the remember feature), or an off-by-default option for the remember feature to support multiple tokens per account.

[Bertg]

We have internally developed a "multiple_remember" feature for rodauth. It looks something like this:

• We created a new table called account_multiple_remember_keys and has a scheme like this: id, account_id, key, deadline
• This value is poured into a cookie as {account_id}_{id}_{convert_token_key(key}. This follows the pattern to expose the ID of the account, and only a single remember token so that brute force attacks on these values can only target one row at a time.
• We delete the cookie AND deleted the remember_key on logout (the cookie is fully validated with timing safe eql before deletion from the DB)
• We delete all remember_keys of an account on account close

Our main motivator for this remains security. The single value shared across multiple devices is still - according to us - a security vulnerability. We imagine a situation where a user signs into browser A and B with remember functionality enabled). These browsers will share the same remember cookie. In our scenario we imagine that device A remains in daily use, but B is not. Even long after the configured deadline the remember cookie on devise B can still be used to restore a session (by a malicious actor) by modifying the expires timestamp in the browser, or directly using the value in a script. I believe that this is similar to a session fixation attack.

Initially we were not going to include backwards compatibility for the old cookie, but we realised that this would sign out a bunch of our users, so that was decided to not be acceptable. So we also update the cookie when we detect an old one. This bit of code will be removed after our configured deadline.

Our code, at this time, is not ready to be turned into a valid PR, as we just focused on our direct needs. (eg, we don't include views as we run API only, and only cover through integration tests).

For reference, here is our implementation:

# frozen-string-literal: true

module Rodauth
  Feature.define(:multiple_remember, :MultipleRemember) do

    before
    before 'load_memory'
    after
    after 'load_memory'
    redirect
    response

    auth_value_method :raw_remember_token_deadline, nil
    auth_value_method :remember_cookie_options, {}.freeze
    auth_value_method :extend_remember_deadline?, false
    auth_value_method :extend_remember_deadline_period, 3600
    auth_value_method :remember_period, {:days=>14}.freeze
    auth_value_method :remember_deadline_interval, {:days=>14}.freeze
    session_key :remember_deadline_extended_session_key, :remember_deadline_extended_at

    auth_value_method :multiple_remember_cookie_key, '_rmmbr'
    auth_value_method :multiple_remember_id_column, :id
    auth_value_method :multiple_remember_account_id_column, :account_id
    auth_value_method :multiple_remember_key_column, :key
    auth_value_method :multiple_remember_deadline_column, :deadline
    auth_value_method :multiple_remember_table, :account_multiple_remember_keys

    auth_value_method :remember_cookie_key, '_remember'
    auth_value_method :remember_id_column, :id
    auth_value_method :remember_key_column, :key
    auth_value_method :remember_deadline_column, :deadline
    auth_value_method :remember_table, :account_remember_keys

    auth_methods(
      :add_remember_key,
      :forget_login,
      :generate_remember_key_value,
      :get_remember_key,
      :load_memory,
      :remembered_session_id,
      :logged_in_via_remember_key?,
      :remember_key_value,
      :remember_login,
      :remove_remember_key
    )

    internal_request_method :remember_setup
    internal_request_method :remember_disable
    internal_request_method :account_id_for_remember_key

    def remembered_session_id
      return unless cookie = _get_remember_cookie
      account_id, key_id, key = cookie.split('_', 3)
      return unless account_id && key_id && key

      actual, deadline = current_remember_key_ds(account_id, key_id).get([multiple_remember_key_column, multiple_remember_deadline_column])
      return unless actual

      if hmac_secret && !(valid = timing_safe_eql?(key, compute_hmac(actual)))
        if hmac_secret_rotation? && (valid = timing_safe_eql?(key, compute_old_hmac(actual)))
          _set_remember_cookie(account_id, key_id, actual, deadline)
        elsif !(raw_remember_token_deadline && raw_remember_token_deadline > convert_timestamp(deadline))
          return
        end
      end

      unless valid || timing_safe_eql?(key, actual)
        return
      end

      account_id
    end

    def remembered_session_key_id
      return unless cookie = _get_remember_cookie
      account_id, key_id, key = cookie.split('_', 3)
      return unless account_id && key_id && key

      return key_id
    end

    def load_memory
      if logged_in?
        if extend_remember_deadline_while_logged_in?
          if account_from_session
            extend_remember_deadline
          else
            forget_login
            clear_session
          end
        end
        # Remove the below code branch once all active sessions in production are updated
        if update_remember_token_to_key_based_remember_token?
          update_remember_cookie

          # Delete the old cookie, no matter what
          opts = Hash[remember_cookie_options]
          opts[:path] = "/" unless opts.key?(:path)
          ::Rack::Utils.delete_cookie_header!(response.headers, remember_cookie_key, opts)
        end
      elsif account_from_remember_cookie
        before_load_memory
        login_session('remember')
        extend_remember_deadline if extend_remember_deadline?
        after_load_memory
      end
    end

    def update_remember_cookie
      return if request.cookies[multiple_remember_cookie_key]

      account_id, key = request.cookies[remember_cookie_key].to_s.split('_', 2)
      return unless account_id && key

      actual, deadline = db[remember_table]
        .where(remember_id_column=>account_id)
        .where(Sequel.expr(remember_deadline_column) > Sequel::CURRENT_TIMESTAMP)
        .get([remember_key_column, remember_deadline_column])

      return unless actual

      actual_matches_key = (hmac_secret && timing_safe_eql?(key, compute_hmac(actual))) || (!hmac_secret && timing_safe_eql?(key, actual))

      if actual_matches_key
        account_from_session
        remember_login
      end
    end

    def update_remember_token_to_key_based_remember_token?
      request.cookies[remember_cookie_key]
    end

    def remember_login
      get_remember_key
      set_remember_cookie
      set_session_value(remember_deadline_extended_session_key, Time.now.to_i) if extend_remember_deadline?
    end

    def get_remember_key
      unless @remember_key_value = active_remember_keys_ds.get(multiple_remember_key_column)
        generate_remember_key_value
        add_remember_key
      end
      nil
    end

    def forget_login
      opts = Hash[remember_cookie_options]
      opts[:path] = "/" unless opts.key?(:path)
      ::Rack::Utils.delete_cookie_header!(response.headers, multiple_remember_cookie_key, opts)
    end

    attr_reader :remember_key_id

    def add_remember_key
      hash = {multiple_remember_account_id_column=>account_id, multiple_remember_key_column=>remember_key_value}
      set_deadline_value(hash, multiple_remember_deadline_column, remember_deadline_interval)
      @remember_key_id = remember_key_ds.insert(hash)
    end

    def remove_remember_key(id=account_id, key_id=remember_key_id)
      remember_key_ds(id).where(multiple_remember_id_column=>key_id).delete
    end

    def remove_all_remember_keys(id=account_id)
      remember_key_ds(id).delete
    end

    def logged_in_via_remember_key?
      authenticated_by.include?('remember')
    end

    private

    def _set_remember_cookie(account_id, key_id, remember_key_value, deadline)
      opts = Hash[remember_cookie_options]
      opts[:value] = "#{account_id}_#{key_id}_#{convert_token_key(remember_key_value)}"
      opts[:expires] = convert_timestamp(deadline)
      opts[:path] = "/" unless opts.key?(:path)
      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, multiple_remember_cookie_key, opts)
    end

    def set_remember_cookie
      _set_remember_cookie(
        account_id,
        remember_key_id,
        remember_key_value,
        current_remember_key_ds(account_id, remember_key_id).get(multiple_remember_deadline_column)
      )
    end

    def extend_remember_deadline_while_logged_in?
      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
      elsif logged_in_via_remember_key?
        # Handle existing sessions before the change to extend remember deadline
        # while logged in.
        true
      end
    end

    def extend_remember_deadline
      return unless cookie = _get_remember_cookie
      account_id, key_id, key = cookie.split('_', 3)
      return unless account_id && key_id && key

      actual, deadline = current_remember_key_ds(account_id, key_id).get([multiple_remember_key_column, multiple_remember_deadline_column])
      return unless actual

      if hmac_secret && timing_safe_eql?(key, compute_hmac(actual))
        current_remember_key_ds(account_id, key_id).update(
          multiple_remember_deadline_column=>Sequel.date_add(Sequel::CURRENT_TIMESTAMP, remember_period)
        )
      end
      _set_remember_cookie(
        account_id,
        key_id,
        actual,
        deadline
      )
      set_session_value(remember_deadline_extended_session_key, Time.now.to_i) if extend_remember_deadline?
    end

    def account_from_remember_cookie
      id = remembered_session_id
      unless id
        # Only set expired cookie if there is already a cookie set.
        forget_login if _get_remember_cookie
        return
      end

      set_session_value(session_key, id)
      account_from_session
      remove_session_value(session_key)

      unless account
        remove_remember_key(id, remembered_session_key_id)
        forget_login
        return
      end

      account
    end

    def _get_remember_cookie
      request.cookies[multiple_remember_cookie_key]
    end

    def after_logout
      forget_current_remember_key
      forget_login
      super if defined?(super)
    end

    def forget_current_remember_key
      return unless cookie = _get_remember_cookie
      account_id, key_id, key = cookie.split('_', 3)
      return unless account_id && key_id && key

      actual, deadline = current_remember_key_ds(account_id, key_id).get([multiple_remember_key_column, multiple_remember_deadline_column])
      return unless actual

      if hmac_secret && timing_safe_eql?(key, compute_hmac(actual))
        current_remember_key_ds(account_id, key_id).delete
      end
    end

    def after_close_account
      remove_all_remember_keys
      super if defined?(super)
    end

    attr_reader :remember_key_value

    def generate_remember_key_value
      @remember_key_value = random_key
    end

    def use_date_arithmetic?
      super || extend_remember_deadline? || db.database_type == :mysql
    end

    def remember_key_ds(id=account_id)
      db[multiple_remember_table].where(multiple_remember_account_id_column=>id)
    end

    def current_remember_key_ds(id=account_id, key_id)
      active_remember_keys_ds(id).where(multiple_remember_id_column=>key_id)
    end

    def active_remember_keys_ds(id=account_id)
      remember_key_ds(id).where(Sequel.expr(multiple_remember_deadline_column) > Sequel::CURRENT_TIMESTAMP)
    end
  end
end

[jeremyevans]

Thanks for submitting the code you are using, I think it will be helpful to others.

Our main motivator for this remains security. The single value shared across multiple devices is still - according to us - a security vulnerability. We imagine a situation where a user signs into browser A and B with remember functionality enabled). These browsers will share the same remember cookie. In our scenario we imagine that device A remains in daily use, but B is not. Even long after the configured deadline the remember cookie on devise B can still be used to restore a session (by a malicious actor) by modifying the expires timestamp in the browser, or directly using the value in a script. I believe that this is similar to a session fixation attack.

This is only true if you set extend_remember_deadline? true, it's not the default behavior. By default, Rodauth does not extend the deadline for a remember token, so the deadline when the token is created is fixed. The deadline is stored in the database, and you try to use a remember token past its deadline, Rodauth will not recognize it as valid.

[Bertg]

This is only true if you set extend_remember_deadline? true, it's not the default behavior.

thanks for the clarification.