Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refresh_ttl config not considered when refreshing the token via RefreshToken middleware #63

Closed
mmichaelbiz opened this issue Mar 14, 2015 · 15 comments
Labels

Comments

@mmichaelbiz
Copy link

Hi Sean,

First off let me say this is a great package!

It seems that the RefreshToken middleware does not consider the 'refresh_ttl' but only the 'ttl' config.

For example set the config as (just for testing):

  • ttl = 1 minute
  • refresh_ttl = 5 mins

I get the new token on every response and use that for the next request which is fine.

But the token can only be used for 1 minute as per the ttl.

I don't want to have the user login again until after 5 mins (as per the refresh_ttl).

What do we need to do to have this work? Have I misunderstood the concept around the refresh_ttl and RefreshToken middleware?

@mmichaelbiz mmichaelbiz changed the title refresh_ttl config not considered when refreshing the token refresh_ttl config not considered when refreshing the token via RefreshToken middleware Mar 14, 2015
@giulioprovasi
Copy link

Have same issue, for now I have done the test myself:

The listener that intercepts the 'expired' token event

    /**
     * Fired when the token has expired
     * @param \Exception $e
     * @return \Illuminate\Http\JsonResponse
     */
    public function expired($e)
    {
        $token = \JWTAuth::parseToken();

        Config::package('tymon/jwt-auth', 'jwt');
        $ttl = Config::get('jwt::refresh_ttl');

        $iat = Carbon::createFromTimestamp($token->getPayload()->get('iat'));
        $now = Carbon::now();

        // if renew ttl is expired too, return 401, otherwise let
        // the application generate a new token to frontend
        if ($iat->diffInMinutes($now) >= $ttl) {
            unset($iat, $now, $ttl);
            return response_failure(
                Lang::get('errors.api.auth.expired'),
                Config::get('status.error.unauthorized')
            );
        }

        unset($iat, $now, $ttl);
    }

The filter

/*
|--------------------------------------------------------------------------
| JWT-Auth token-refresh Filter
|--------------------------------------------------------------------------
|
| The RefreshToken filter update the response headers by returning an 
| updated authentication token.
|
*/
Route::filter('RefreshToken', function($route, $request, $response)
{
    $token = JWTAuth::parseToken();

    try {
        $token->toUser();
    } catch (TokenExpiredException $e) {
        Config::package('tymon/jwt-auth', 'jwt');
        $ttl = Config::get('jwt::refresh_ttl');

        $iat = \Carbon\Carbon::createFromTimestamp($token->getPayload()->get('iat'));
        $now = \Carbon\Carbon::now();

        if ($iat->diffInMinutes($now) < $ttl) {
            $response->headers->set('Authorization', 'Bearer ' . $token->refresh());
        }
    }
});

And any authenticated route

Route::group(['before' => 'jwt-auth', 'after' => 'RefreshToken'], function () { ... });

I still hate a problem anyway, when the token is refreshed the first time, it's possible that some other requests are pending asynchronously, so when the first one resolves and get back with the updated token, all the following raise a 500err because their token have been blacklisted... any ideas ?

@mattmcdonald-uk
Copy link
Contributor

I have the same issue with asynchronous requests when using the refresh method.

I'm also having problems with parsing the token to get a user. The refresh middleware runs before the controller runs, and I then use this within the controller:

JWTAuth::parseToken() -> toUser();

This throws a blacklisted exception, presumably because the token just got blacklisted by the refresh middleware?

I looked at the source, and it seems to want to wait 60 seconds (or minutes?) before enforcing a blacklist, but this doesn't appear to be happening in reality.

This is all with the latest Laravel 5

@giulioprovasi
Copy link

Same problem here with laravel 4.2

@tymondesigns
Copy link
Owner

@mmichaelbiz Thanks for highlighting this. It looks like I need to look at the refresh flow properly, and I guess I need to bulk out the token refreshing tests aswell.

Apologies for the slow response, been mega busy lately.

Thanks! 👍

@mmichaelbiz
Copy link
Author

No sweat. I have an angular service set up and listening out for any refreshed tokens so as soon as you get it sorted let me know to test it out!

On another note, how do we handle cross device tokens? i.e. User logs in via their desktop browser and gets a JWT, then logs in via their mobile / tablet. Should be able to get a new token for the additional device without invalidating the first.

@artemverbo
Copy link

+1 please, having similar issues with L5 and Angular front-end. Thank you, great project!

@DangerLifter
Copy link

+1 please, having similar issues with L5

@DangerLifter
Copy link

As I see JWTManager::refresh() Should call setRefreshFlow() before decode to prevent "expire" validation.

@tymondesigns
Copy link
Owner

@mmichaelbiz Just pushed out 0.5.1, think this might solve the issue (was more concerned about the security issue raised here)

don't have time to test it properly right now, but feel free to give it a go 👍

@mmichaelbiz
Copy link
Author

Awesome, thanks for the heads up. Will aim to give it a go this week!

@mmichaelbiz
Copy link
Author

@tymondesigns, I took some time to test this out:

  1. When using the refresh middleware there is something blacklisting the refreshed token that is sent, and on subsequent requests it gets blocked. (similar to as mentioned by @mattmcdonald-uk)
    I commented out JWTManager.php line 70 to work around this in order to continue tests.
  2. Initial test have proven successful however in that you can keep making refresh calls up until the refresh_ttl. 👍
  3. Note that the refresh_ttl is not being loaded from the published config file however and I had to change the $refreshTTL variable directly on the PayloadValidator to test this.
  4. On a general note, and I think you have mentioned this before, would be good to sort out some of the error handling and response messages. (e.g. Error thrown in PayloadValidator line 89 with a great message but that message is not passed through to the response and all we get is "token_expired")

Would be nice to be able to add our own HTTP Status Codes, messages and possibly even Error Codes for the various exceptions. (i'll link this it #67)

👍 on the security fix by the way. I see you have blocked none algorithms completely.

@tymondesigns
Copy link
Owner

@mmichaelbiz Thank you for the feedback!

I'm about to spin up a fresh install, so I will look into the issues you raised

Thanks! 👍

@lewiswharf
Copy link

@tymondesigns I have found similar re: @mmichaelbiz points 1 & 3.

Thank you! 👍

@jfadich
Copy link
Contributor

jfadich commented Apr 23, 2015

This issue should be fixed in pull request #102 as @DangerLifter suspected, the setRefreshFlow wasn't set before the token was decoded.

@tymondesigns
Copy link
Owner

Closing as main issue resolved (as of 0.5.3), will be looking at custom responses/status codes soon

agungsugiarto pushed a commit to agungsugiarto/jwt-auth that referenced this issue Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants