0.1.43 does not return client and access-token with each response

[jeffchuber]

The latest version now wipes access-token and client for subsequent API requests:

https://github.com/lynndylanhurley/devise_token_auth/blob/00fa5f4507da806f66664787745086ce404d95b4/app/controllers/devise_token_auth/concerns/set_user_by_token.rb#L18

# keep track of request duration
  def set_request_start
    @request_started_at = Time.zone.now
    @used_auth_by_token = true

    # initialize instance variables
    @client_id = nil
    @resource = nil
    @token = nil
    @is_batch_request = nil
  end

The way we had our application set up was relying on those access tokens being returned as they were in previous releases.

Why was this change added? The commit only mentions passing tests: https://github.com/lynndylanhurley/devise_token_auth/commit/bdcd05e284e7fd0774005248a3239bfc889824c4

[MaicolBen]

@jeffchuber Do you say that setting those values in nil (in a before_action) could have wiped out the tokens? Because that doesn't reset the user's tokens, if you see the line 136, it extends the batch buffer. Maybe the problem is #703

[jeffchuber]

Hi @MaicolBen , thanks for the reply!

Here is what I know:

1. 0.1.43 does not return access-token and client
2. I can bundle open devise_token_auth and comment out the lines like this:

def set_request_start
    @request_started_at = Time.now
    @used_auth_by_token = true

    # initialize instance variables
    # @client_id = nil
    # @resource = nil
    # @token = nil
    # @is_batch_request = nil
  end

3. and API now returns access-token and client in the headers.

(I could modify my iOS app to handle this new case - but we have a broad install base, and this is a breaking change :/ )

Any ideas?

[twolfson]

Hey @MaicolBen, I'm a coworker of @jeffchuber. I think this issue was initially reporting the would-be solution as opposed to the issue we're seeing

We've been using the API as follows:

module Api
  class BaseController < ApplicationController
    include DeviseTokenAuth::Concerns::SetUserByToken
    before_action :authenticate_user!
  end
end

This is my first time working with this module so I'm not sure if this is the ideal setup. Based on the docs though, it does seem accurate

Unfortunately, this was working before 0.1.43 and is no longer working afterwards

The problem seems to be that set_user_by_token is being called before set_request_start (verified via puts at top of each method). For my own reference, set_user_by_token is being run by authenticate_user! which is provided by helpers.rb#devise_token_auth_group

https://github.com/lynndylanhurley/devise_token_auth/blob/v0.1.43/lib/devise_token_auth/controllers/helpers.rb#L33-L45

Additionally, this might be caused by us using Rails 4 instead of Rails 5 so maybe before_action aren't guaranteed with a specific order. Unfortunately, rearranging the include/before_action doesn't seem to help the issue either

[twolfson]

As a workaround for now, we're going to update the nil lines to the following:

def set_request_start
    @request_started_at = Time.now
    @used_auth_by_token = true

    # initialize instance variables
    @client_id ||= nil
    @resource ||= nil
    @token ||= nil
    @is_batch_request ||= nil
  end

[devdudeio]

Since the update was shipped the client only got a new access-token when the used access-token was expired. So even when I used the access-token 100 times in a row (batched) i get no new access-token.

When the client waits for 5 seconds (default) the next response will give me a new access-token. The Problem here is that used and expired tokens are still valid to use a last time for 2 weeks (default) instead of only 5 seconds (normally you have to use the next access-token from the response here)

Before the 1.43 update the access-token was only valid for 2 weeks when it was not used before. And the client got a new access-token for every request it makes.

This is a breaking change for sure.

Workaround for clients: if you have a client check if your request gives you an empty access-token. If its empty your old token is still valid and not expired so you can use it for the next query. When your access-token is expired, your next (and last possible query with that access-token) will give you the next valid access-token

[MaicolBen]

@twolfson can you make a PR with that workaround? It seems to me the same as commenting the variables, but maybe it can throw undefined variable.

[twolfson]

Sure thing! Opening a PR now 👍

[krzysiek1507]

It looks like #703 #1161

[MaicolBen]

Thanks @krzysiek1507 . @twolfson can you check if #1161 works for you?

[twolfson]

We aren't using batch mode so it's irrelevant to us =/

[twolfson]

Oh, you mean try out #1161 since #703 is landed. I'm quite cramped this week so I don't have time to try out the other fixes. Sorry =/

[twolfson]

Feel free to close #1166 if you feel #1161 would resolve this issue

[MaicolBen]

If you aren't using batch mode, it's a different issue.

[krzysiek1507]

IIRC, every 2 requests made in batch_request_buffer_throttle seconds 'enable' batch mode so even if you log in (1st request) and go do home page (2nd request) then gem in version 0.1.43 treats it as batch mode and returns empty token.

[Eth3rnit3]

I am in the same case as jeffchuber, except that I installed the gem only 10 days ago and after validation, I no longer receive tokens even if the initializer is set to true, but with the jeffchuber hack it works. I am currently on rails 5.2 but the app was initiated with 5.0.

[devdudeio]

@Eth3rnit3 use the token once. Wait 5 seconds and use it again. New token will appear in the response.

New tokens will only appear when the old token was used min. 1x and then again after x seconds (default 5)

[Eth3rnit3]

No I do the test with postman from one side and react-native from the other and in both cases after validation no more token. But for me it's not a big deal, I'm at the beginning of my app and I created my own logic for the token storage, so I planned the case where the token is empty, I keep the previous one.

FYI, I did another test with an app with a 5.1.5 version of rails and no problem I have a token each response

thank you for your reply @rlech

[iCode4life]

my workaround for this bug, i simply changed devise_token_auth code and i made it to generate new token and kill the old one

[dks17]

I think the reason is that access-token header with empty value is returned in the batch mode. Clients, e.g. ng-token-auth awaits the same access-token value in each batch server responses. https://github.com/lynndylanhurley/devise_token_auth/blob/470c84beabff3ca59c239abe900d43d7750d0d6a/app/controllers/devise_token_auth/concerns/set_user_by_token.rb#L165-L171

Note that when the server identifies that a request is part of a batch request, the user's auth token is not updated. The auth token will be updated for the first request in the batch, and then that same token will be returned in the responses for each subsequent request in the batch (as shown in the diagram).

If you want return back old behaviour of the gem try to set batch_request_buffer_throttle to 0.

@MaicolBen Do you plan revert #703 PR with #1161 PR for master branch and upcoming release? I suppose this issue was only resolved in 0.2.0 release.

[MaicolBen]

@dks17 No, should we? I am planning to make change_headers_on_each_request disabled as default in 1.1.0, it is causing more headaches than the security enhancement.

[dks17]

@MaicolBen I think we should. Response is returned with Cache-Control header equals max-age=0, private, must-revalidate. We may use Cache-Control header.

And replace this code: https://github.com/lynndylanhurley/devise_token_auth/blob/470c84beabff3ca59c239abe900d43d7750d0d6a/app/controllers/devise_token_auth/concerns/set_user_by_token.rb#L165-L171 by something like this:

response.headers['Cache-Control'] = 'no-cache'
# or
auth_header['Cache-Control'] = 'no-cache'

Thus we return default behaviour back, return compatibility with clients, and resolve #702 issue.

Didn't check, but should work.

[MaicolBen]

@dks17 Your approach seems neat! can you make a PR?

[dks17]

@MaicolBen It doesn't look difficult. But it should be covered with tests with consider browsers behaviour which described in #702. I will back to the issue if I have an idea how to test it.

Or we just revert #703 PR.

[dks17]

@MaicolBen

I am planning to make change_headers_on_each_request disabled as default in 1.1.0

If you do it I suspect that user will log out right after the expiration of a token ends. This will happen every time with token_lifespan frequency. And when a token was issued does not matter.

Whose responsibility is to follow the expiration of tokens (server or client)?

[MaicolBen]

If you do it I suspect that user will log out right after the expiration of a token ends.

Yes, I guess you meant "login". I don't know yet if I will make the change, I don't have a lot of time to contribute to this repo and I haven't used this gem for some time :(. I think a lot of people is using change_headers_on_each_request disabled (including myself in the past) so it seems reasonable as a default but it goes against the goal of the creation of this gem

Whose responsibility is to follow the expiration of tokens (server or client)?

You answered the question, the server will expire after the token lifespan has passed.

[dks17]

You answered the question, the server will expire after the token lifespan has passed.

I meant a little different.

I think it is possible add accessory action like reissue_token to controller. Client sends request and server returns response with new token. In this case a client must control a token expiry and sends request when it is needed (when client sees that a token expiry almost elapsed). This approach will require support by clients.

We will have two honest approach of token exchanging: change token each request and refresh token by client's demand.

[MaicolBen]

@dks17 sounds good!

[aymenbuu]

Any updates on this?

I am tempted to go with the workaround propositioned by @dude-awesome

Workaround for clients: if you have a client check if your request gives you an empty access-token. If its empty your old token is still valid and not expired so you can use it for the next query. When your access-token is expired, your next (and last possible query with that access-token) will give you the next valid access-token

Any reasons why I shouldn't?

Running with devise_token_auth (1.1.4, 1.1.3)

[bettysteger]

Any updates on this? i got users that get logged out randomly after some time - but i am not sure if this is the best configuration with v1.2.0:

  config.change_headers_on_each_request = true
  config.token_lifespan = 4.weeks
  config.batch_request_buffer_throttle = 2.minutes