Token invalidation after canceled request by the frontend app

[LeoValverde]

I'm using devise token auth and implementing my own logic at the front-end to handle the token management.

After many of my users reporting a sudden token invalidation, I found out that this is caused when the frontend makes the request and cancels it (it happens when we refresh the browser in the middle of the promise runtime).

When the backend generates a new token but the frontend app is no longer listening to the response, the frontend only has the "last token" to work with instead of the newest one. After five seconds, the old token expires and the user is kicked out to the login page.

Any ideas to solve this?

Considering I could implement any logic in the frontend to put the authentication back on track after this event, I would still need more functionality in the devise token auth lib?

[MaicolBen]

@LeoValverde Maybe you can try increasing batch_request_buffer_throttle. Doing some investigation, we cannot know with http 1.0/1.1 if the client gets disconnected. Another solution is disabling change_headers_on_each_request

[LeoValverde]

Thank you for the response, @MaicolBen! After the moment the frontend didn't recieve the most recent token, it's a matter of time for it to get disconnected, since the devise token auth doesn't repeat the most recent token.

Increasing the batch_request_buffer_throttle would not avoid the problem to happen, but only postpone the error for a few more seconds.

[rmcsharry]

@LeoValverde Thank you for identifying this problem and writing it up here. I was going nuts trying to figure out why I keep getting unauthorized responses.

There are various scenarios that cause this and it seems to have been around since Oct 2014.
See here for more info and an open StackOverflow question here.

I have added 100 bounty to the question.

[rmcsharry]

I found another SO question here which talks about a totally different solution, where refresh tokens are stored and they are used to issue the actual jwt access token (which is not stored). This is also the solution used by Auth0, explained here.

This would require a fairly significant modification of this gem and would possibly be a breaking change...?

[LeoValverde]

@rmcsharry I've decided to adopt a simple solution that seems to be working very well. I'm not sure if it is the best way to workaround this issue, but it is the best I've got so far. After understanding how the devise token auth code, I've decided to override a small part of its behavior to get rid of the problem.

Background

Devise token auth works with two tokens per user/client: the main one, called "token" which is the most fresh and the one that causes token rotation when sent back to the server; and the "last_one", which is valid for a few seconds (5 by default). Token rotation only happens when the token sent is the most fresh one.

The token rotation works like that:
(new token) -> token -> last_token -> (discarded)

The intention is to consider that there is one valid token that must be used to keep the normal token lifecycle going, but, in order to prevent network latency (and packages getting to the server in a different order), the last_token is a tolerance added to keep the last valid token as valid for a few more seconds.

The concept of the solution

I assumed that it is super unlikely that the user will to cancel (by refreshing or closing the browser) a pending request that would come back with a new token 2 times in a row.

The idea is to create an intermediary token, which I called "previous_token", that works exactly like the main one (the so called "token"). This means that it should rotate when it's recieved by the server and it should have the same validity of the main token.

After the change, token rotation should work like that:
(new token) -> token -> previous_token -> last_token -> (discarded)

This change would keep two tokens as valid (instead of one), and another one as a small tolerance, exactly as it were before.

The code

To implement this change, I just had to override two methods of the DeviseTokenAuth::Concerns::User file. Because this is included in my User model class, I had to change these methods using monkey patching.

If the solution proves that it is stable and that it solves the problem, I think I'll try to reach the author of the library to explain the problem e propose the solution.

• User model BEFORE the change

class User < ActiveRecord::Base
  include DeviseTokenAuth::Concerns::User

  #- there is a lot more code in here that doesn't matter
end

• User model AFTER the change

class User < ActiveRecord::Base
  include DeviseTokenAuth::Concerns::User

  #- there is a lot more code in here that doesn't matter
end

class User
  def token_is_current?(token, client_id)
    # ghetto HashWithIndifferentAccess
    expiry     = tokens[client_id]['expiry'] || tokens[client_id][:expiry]
    token_hash = tokens[client_id]['token'] || tokens[client_id][:token]
    previous_token_hash = tokens[client_id]['previous_token'] || tokens[client_id][:previous_token]

    return true if (
      # ensure that expiry and token are set
      expiry && token &&

      # ensure that the token has not yet expired
      DateTime.strptime(expiry.to_s, '%s') > Time.zone.now &&

      # ensure that the token is valid
      (
        (token.present? && DeviseTokenAuth::Concerns::User.tokens_match?(token_hash, token)) or
        (previous_token_hash.present? && DeviseTokenAuth::Concerns::User.tokens_match?(previous_token_hash, token))
      )
    )
  end

  # update user's auth token (should happen on each request)
  def create_new_auth_token(client_id = nil)
    now = Time.zone.now

    client_id, token = create_token(
      client_id: client_id,
      expiry: (now + token_lifespan).to_i,
      previous_token: tokens.fetch(client_id, {})['token'],
      last_token: tokens.fetch(client_id, {})['previous_token'],
      updated_at: now
    )

    update_auth_header(token, client_id)
  end
end

The change may seem big, but it is actually a copy paste of the two methods with a few small changes.

I hope it works for you. Please, share your opinion about this alternative here.

PS: I've seen in one of the links you shared that making two tokens valid at the same time would be a security issue. I'm not sure why is that.

[nathanmauro]

any action on this?

[LeoGardel]

I would like to hear from @lynndylanhurley. Maybe there is a better solution to this or perhaps I should make a pull request with this alternative I presented...

[rmcsharry]

@LeoValverde Thanks for that detailed explanation and solution. It makes total sense. I'm afraid it will be a couple of week before I can implement and test this.

Having 2 tokens valid simultaneously is a security issue in case of hijacking - a hijacker could use to generate a new actual token (eg. that would then be valid for 2 weeks, or however long your validity period is for the JWT).

However, in practice, I would argue that

1. using SSL it will be almost impossible to hijack one of the tokens and
2. if someone was that determined to 'hack' your api and succeeded, I think you have far bigger problems than 2 simultaneously valid tokens!

I would love to hear from @lynndylanhurley on your proposed solution also.

[rrjohnson85]

Has there been any movement on this at all?

[lynndylanhurley]

@LeoValverde sorry for the late response to this. Interesting idea - did your solution completely eliminate the problem?

Seems like the security impact of this fix is marginal. The main security benefit of cycling the tokens is that the user will know quickly if their session has been hijacked, because their current session will be invalidated. I don't think we lose that with this fix.

[LeoValverde]

@lynndylanhurley my fix reduced A LOT the number of unauthorized responses, and made me able to keep using DTA in production. Even though, a few users still reporting sudden disconnections, and I think they are IPhone users. I really don't know why this happens to these people, but this problem moved from the top 1 bug and most important problem to solve, to something that annoys a really small portion of my users.

In my humble opinion, this fix should be integrated to DTA, at least while we don't come up with a better and definitive solution.

[lynndylanhurley]

@LeoValverde - thanks for that feedback. Can you send a PR? If so I'll test and review ASAP

[LeoValverde]

@lynndylanhurley I'll do it ASAP!

[LeoValverde]

@lynndylanhurley I did it
#1315

Thank you for addressing the problem.

[bananatron]

Fwiw my method for testing this was creating a super simple page that requests from our API on an interval that is slightly higher than the batch_request_buffer_throttle and sets the headers for future requests using localstorage.

If I run this in multiple tabs (I usually do about 3), inevitably I hit a 401 on one tab, and all subsequent requests continue to work on all tabs. Keeping the network pane on chrome open is helpful in seeing the trail of requests. I can't be 100% sure it's the same issue as others are having and because it seems to be timing related, I can't reliability reproduce it.

It certainly seems like having 2+ validatable tokens would fix this, but it seems hard to know since I can't find a way to reproduce on demand.

[yoshitsugu]

@LeoValverde @MaicolBen
is there any updates for this PR? #1315

[sudhanshug16]

Added a PR for this. Review requested @MaicolBen @lynndylanhurley