JWTRefreshTokenBundle icon indicating copy to clipboard operation
JWTRefreshTokenBundle copied to clipboard

Question: refreshing invalid JWT with valid refresh token?

Open sebastianstucke87 opened this issue 2 years ago • 14 comments

Hello,

According to your documentation, the JWT must still be valid in order to be renewed (with a valid refresh token ofc):

Ask to renew valid JWT with our refresh token. Make a POST call to (...)

(via https://github.com/markitosgv/JWTRefreshTokenBundle#generating-tokens)

Is that correct? Because if so, oh boy! That would be quite a bummer... 😅

sebastianstucke87 avatar Apr 13 '22 18:04 sebastianstucke87

Nah, that's just bad wording. As long as the refresh token is valid then you're good; on a successful refresh, the refresh endpoint generates a new JWT.

mbabker avatar Apr 15 '22 17:04 mbabker

Are you sure about that? Because I'm struggling at the moment with this: If I call the refresh route with a valid-but-expired JWT token, and a valid and not expired refresh token, I get a 401 ExpiredJWT token because in this case, gesdinet.refresh_token is never called.

With the exact same API code, if I now call the refresh route with a valid and not expired JWT token, gesdinet.refresh_token is called, and everything works fine...

EDIT: I have found the reason. If you send a request with "Authorization: Bearer xxxxx" in it, no matter if you route is declared as "PUBLIC_ACCESS" in the security.yaml, lexik/JWT will test the JWT token and reject it if it is expired.

Of course, the solution is easy: don't send the token if it is expired. But what if -like in my case- we want to force the user to send a token to make sure he has a valid token , minus the fact it is expired? It would be be way secure this way in my opinion, since you don't only need one, but two different tokens (and likely stored to different places) The fact that we can obtain a new JWT token by providing only a refresh token sounds a bit weak to me...

geoffroyp avatar May 09 '22 08:05 geoffroyp

An expired JWT can’t be considered valid in the same way an expired session can’t be resumed.

Very loosely, the refresh token can be compared to a remember me cookie; it’s only issued after the user authenticates their self to the application, and it is only valid for that user based on how you configure the remember me feature.

mbabker avatar May 09 '22 11:05 mbabker

Related https://github.com/markitosgv/JWTRefreshTokenBundle/issues/334

webda2l avatar Jul 27 '22 15:07 webda2l

I have indeed doubts about the security of this system... I can create new JWT only with the refresh token. Let's imagine that my database leaks, anyone can connect with the account of each of my users, just by using the refresh token found in the database.

He just has to go to /api/token/refresh, give the refresh token, and he has a valid JWT ! I don't understand the use of hashing the passwords if it's to display the refresh tokens in clear.... Maybe a hash of the tokens in database would be a good thing. Only the user who received the refresh token in clear text during his connection can use it...

AimSai59 avatar Aug 05 '22 04:08 AimSai59

I have indeed doubts about the security of this system...

If your database leaks, the refresh token values being stored in plaintext is the least of your concerns.

While #289 would be a better spot to chime in with your concerns, let me explain the problem here.

The problem with trying to hash refresh tokens is there is no way to match them up with a user. On a refresh request, the only thing you have to try and authenticate the user with is the refresh token. It's not something like a JWT where the token itself contains all of the needed information to authenticate, and it's not like logging in with a username and password combo where the username is provided in the request and you can compare hashed passwords. So you have to be able to query a value to get a token record. To store hashed tokens, you'd need something that produces a consistent hashed value, so you can't use something like BCrypt or Argon2 like you would with a password. Maybe something like hash('sha384', $token); can work here, but there'd be a fair bit of effort involved to support hashing tokens (not to mention the major B/C impact there would be to go from plaintext storage to hashed).

mbabker avatar Aug 05 '22 14:08 mbabker

I found a way to hash the tokens via sha-256 here: https://github.com/markitosgv/JWTRefreshTokenBundle/issues/289#issue-1067138994 even if the level of protection is not as important as the effort put on the passwords, it remains, in my opinion, more secure. This doesn't take away the fact that for any sensitive operation, I will have to ask again for the login credentials, not to let a simple token owner modify sensitive information.

I would have thought that asking, in addition to the refresh token, at least a correctly signed JWT token, even expired, containing the username, would be a much better security, knowing that we could limit the age of the acceptable JWT token to the date of the refresh token creation...

AimSai59 avatar Aug 05 '22 14:08 AimSai59

I would have thought that asking, in addition to the refresh token, at least a correctly signed JWT token, even expired, containing the username, would be a much better security, knowing that we could limit the age of the acceptable JWT token to the date of the refresh token creation...

LexikJWTAuthenticationBundle's services won't let you decode an expired JWT. So requiring a valid JWT along with the refresh token isn't an option unless you're going to rewire that bundle to not hard abort decoding an expired token.

mbabker avatar Aug 05 '22 14:08 mbabker

In fact, that's not quite true. By the way, I successfully secured my Refresh Tokens:

  • By hashing the tokens in the database
  • By including my Refresh Token inside the JWT token
  • By using the Authorization : Bearer ..... header to get the refresh token if the JWT token is valid, even expired
  • By checking that the username contained in the JWT token is the same as the one associated with the Refresh Token

To do this, you must first add this to security.yaml (see https://github.com/markitosgv/JWTRefreshTokenBundle/issues/307#issuecomment-1207537320): This will prevent route errors, and it will be necessary afterwards.

firewalls:
    refresh:
        // ...
        refresh_jwt: 
            check_path: gesdinet_jwt_refresh_token

Then, we follow the indications of https://github.com/markitosgv/JWTRefreshTokenBundle/issues/289#issue-1067138994 to hash the token just before its recording in database, and just at the time of its reading. We must use an algorithm that always returns the same string for the same entry (here sha-256) :

We create a hash tool:

<?php

namespace App\Api\Security\Tool;


class Hasher
{
    public function hash(string $secret): string
    {
        return hash('sha256', $_ENV['APP_SECRET'] . $secret);
    }
}

We will extend the gesdinet.jwtrefreshtoken.refresh_token_manager service to rewrite the get method to hash the provided token before performing a database search :

<?php

namespace App\Api\Security\Tokens\RefreshToken;

use App\Api\Security\Tool\Hasher;
use Gesdinet\JWTRefreshTokenBundle\Doctrine\RefreshTokenManager;

class RefreshTokenHash extends RefreshTokenManager
{
    public function get($refreshToken)
    {
        return parent::get(
            (new Hasher)->hash($refreshToken)
        );
    }
}

Then, we must replace the class for this service in services.yaml :

gesdinet.jwtrefreshtoken.refresh_token_manager:
    class: App\Api\Security\Tokens\RefreshToken\RefreshTokenHash
    arguments:
        - '@gesdinet.jwtrefreshtoken.object_manager'
        - '%gesdinet.jwtrefreshtoken.refresh_token.class%'

To hash the token in base, we subscribe to the Doctrine pre and post-perstit events :

<?php

namespace App\Api\Security\EventSubscriber;

use Doctrine\ORM\Events;
use App\Api\Security\Tool\Hasher;
use Doctrine\ORM\Event\LifecycleEventArgs;
use Gesdinet\JWTRefreshTokenBundle\Entity\RefreshToken;
use Doctrine\Bundle\DoctrineBundle\EventSubscriber\EventSubscriberInterface;

class RefreshTokenSubscriber implements EventSubscriberInterface
{
    private array $cleartexts = [];

    public function __construct(private Hasher $hasher)
    {
    }

    public function getSubscribedEvents(): array
    {
        return [
            Events::prePersist,
            Events::postPersist,
        ];
    }

    public function prePersist(LifecycleEventArgs $args)
    {
        $entity = $args->getObject();

        if (!$entity instanceof RefreshToken) {
            return;
        }

        $this->hashBeforeSaving($entity);
    }

    public function postPersist(LifecycleEventArgs $args)
    {
        $entity = $args->getObject();

        if (!$entity instanceof RefreshToken) {
            return;
        }

        $this->makeClearBeforeUsing($entity);
    }

    private function hashBeforeSaving(RefreshToken $refreshToken)
    {
        if (128 !== strlen($refreshToken->getRefreshToken())) {
            return;
        }

        $this->cleartexts[spl_object_hash($refreshToken)] = $refreshToken->getRefreshToken();

        $refreshToken->setRefreshToken($this->hasher->hash($refreshToken));
    }

    private function makeClearBeforeUsing(RefreshToken $refreshToken)
    {
        if (!($this->cleartexts[spl_object_hash($refreshToken)] ?? false)) {
            return;
        }

        $refreshToken->setRefreshToken($this->cleartexts[spl_object_hash($refreshToken)]);

        unset($this->cleartexts[spl_object_hash($refreshToken)]);
    }
}

For the next step, we will need 2 classes allowing to extract the tokens.

The first one will get the JWT token in the headers and extract its payload thanks to the service Lexik\Bundle\JWTAuthenticationBundle\Services\JWTTokenManagerInterface, even if the token is expired. For that, we parse the JWT token and we intercept the errors. If the error reason is not expired_token, the error is reported, otherwise, the payload is in the error and can be read with getPayload().

<?php

namespace App\Api\Security\Tokens\JWTToken;

use Lexik\Bundle\JWTAuthenticationBundle\Exception\JWTDecodeFailureException;
use Symfony\Component\HttpFoundation\Request;
use Lexik\Bundle\JWTAuthenticationBundle\Services\JWTTokenManagerInterface;
use Throwable;

class JWTTokenExtractor
{
    public function __construct(private JWTTokenManagerInterface $JWTManager)
    {
    }

    public function extract(Request $request)
    {
        if (!$request->headers->has('authorization')) {
            return false;
        }

        $authorizationHeader = $request->headers->get('authorization');

        $headerParts = explode(' ', (string) $authorizationHeader);

        if (!(2 === count($headerParts) && 0 === strcasecmp($headerParts[0], 'Bearer'))) {
            return false;
        }

        try {
            return $this->JWTManager->parse($headerParts[1]);
        } catch (Throwable $e) {
            /** @var JWTDecodeFailureException $e */
            if ($e->getReason() !== 'expired_token') {
                throw new JWTDecodeFailureException($e->getReason(), $e->getMessage(), null, null);
            } else {
                return $e->getPayload();
            }
        }
    }
}

The second is to add a new extractor to JWTRefreshTokenBundle which will be used to find the Refresh Token in the headers, using first the JWTTokenExtractor class created just before.

<?php

namespace App\Api\Security\Tokens;

use Throwable;
use Symfony\Component\HttpFoundation\Request;
use App\Api\Security\Tokens\JWTToken\JWTTokenExtractor;
use Gesdinet\JWTRefreshTokenBundle\Request\Extractor\ExtractorInterface;

class JWTRefreshTokenExtractor implements ExtractorInterface
{
    public function __construct(private JWTTokenExtractor $JWTTokenExtractor)
    {
    }
    public function getRefreshToken(Request $request, string $parameter): ?string
    {
        try {
            $JWT = $this->JWTTokenExtractor->extract($request);
        } catch (Throwable $e) {
            return null;
        }

        if ($JWT && is_array($JWT) && array_key_exists($parameter, $JWT)) {
            return $JWT[$parameter];
        } else {
            return null;
        }
    }
}

It is now that the magic happens...

First of all, we are going to create a new exception to manage the case where the username linked to the refresh token doesn't match the one of the connected user (and thus of the JWT token) :

<?php

namespace App\Api\Security\Exception;

use Symfony\Component\Security\Core\Exception\AuthenticationException;

class UserNotMatchException extends AuthenticationException
{
    public function getMessageKey()
    {
        return 'The user name in the JWT token does not match with the one associated to the refresh token';
    }
}

The adding of the Refresh Token to the return of the connection with Lexik is done on the event lexik_jwt_authentication.on_authentication_success but to add information to the JWT token, it is necessary to look at the event lexik_jwt_authentication.on_jwt_created.

The service which manages this addition is gesdinet.jwtrefreshtoken.send_token (Gesdinet\JWTRefreshTokenBundle\EventListener\AttachRefreshTokenOnSuccessListener) So we'll replace the class Gesdinet\JWTRefreshTokenBundle\EventListener\AttachRefreshTokenOnSuccessListener by a custom class for this service, and listen to the 2 events. In services.yaml, we add :

gesdinet.jwtrefreshtoken.send_token:
    class: App\Api\Security\EventListener\AttachRefreshTokenOnSuccessListener
    arguments:
        - '@gesdinet.jwtrefreshtoken.refresh_token_manager'
        - '%gesdinet_jwt_refresh_token.ttl%'
        - '@request_stack'
        - '%gesdinet_jwt_refresh_token.token_parameter_name%'
        - '%gesdinet_jwt_refresh_token.single_use%'
        - '@gesdinet.jwtrefreshtoken.refresh_token_generator'
        - '@gesdinet.jwtrefreshtoken.request.extractor.chain'
        - '%gesdinet_jwt_refresh_token.cookie%'
        - '%gesdinet_jwt_refresh_token.return_expiration%'
        - '%gesdinet_jwt_refresh_token.return_expiration_parameter_name%'
    tags:
        - 'kernel.event_listener' :
            event: 'lexik_jwt_authentication.on_jwt_created'
            method: 'attachRefreshTokenToJWT'
        - 'kernel.event_listener' :
            event: 'lexik_jwt_authentication.on_authentication_success'
            method: 'attachRefreshToken'

Because of protected/private properties, we can't just decorate the gesdinet.jwtrefreshtoken.send_token service so we'll have to repeat the constructor with all its arguments.

<?php

namespace App\Api\Security\EventListener;

use Jenssegers\Agent\Agent;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Http\HttpUtils;
use Symfony\Component\HttpFoundation\RequestStack;
use App\Api\Security\Exception\UserNotMatchException;
use Symfony\Component\Security\Core\User\UserInterface;
use Gesdinet\JWTRefreshTokenBundle\Model\RefreshTokenInterface;
use Lexik\Bundle\JWTAuthenticationBundle\Event\JWTCreatedEvent;
use Gesdinet\JWTRefreshTokenBundle\Model\RefreshTokenManagerInterface;
use Gesdinet\JWTRefreshTokenBundle\Request\Extractor\ExtractorInterface;
use Lexik\Bundle\JWTAuthenticationBundle\Event\AuthenticationSuccessEvent;
use Gesdinet\JWTRefreshTokenBundle\Generator\RefreshTokenGeneratorInterface;
use Gesdinet\JWTRefreshTokenBundle\EventListener\AttachRefreshTokenOnSuccessListener as EventListenerAttachRefreshTokenOnSuccessListener;

class AttachRefreshTokenOnSuccessListener extends EventListenerAttachRefreshTokenOnSuccessListener
{

    private ?string $token = null;

    public function __construct(
        RefreshTokenManagerInterface $refreshTokenManager,
        $ttl,
        RequestStack $requestStack,
        $tokenParameterName,
        $singleUse,
        RefreshTokenGeneratorInterface $refreshTokenGenerator,
        ExtractorInterface $extractor,
        array $cookieSettings,
        bool $returnExpiration,
        string $returnExpirationParameterName,
        private EntityManagerInterface $em,
        private HttpUtils $httpUtils,
    ) {
        parent::__construct(
            $refreshTokenManager,
            $ttl,
            $requestStack,
            $tokenParameterName,
            $singleUse,
            $refreshTokenGenerator,
            $extractor,
            $cookieSettings,
            $returnExpiration,
            $returnExpirationParameterName
        );
    }

    public function attachRefreshTokenToJWT(JWTCreatedEvent $event): void
    {
       // lexik_jwt_authentication.on_jwt_created event
    }

    public function attachRefreshToken(AuthenticationSuccessEvent $event): void
    {
        // lexik_jwt_authentication.on_authentication_success event
    }
}

The goal is to use the attachRefreshToken() method of the Gesdinet\JWTRefreshTokenBundle\EventListener\AttachRefreshTokenOnSuccessListener class which creates the Refresh token and adds it to the return of the Lexik authentication and then link it to the lexik_jwt_authentication. on_jwt_created event rather than to lexik_jwt_authentication.on_authentication_success. This way we can add the refresh token IN the JWT token.

The problem is that the cookie saving (if enabled) depends on information contained exclusively in the AuthenticationSuccessEvent object. So we have to split the method in two.

As the events are emitted in this order: lexik_jwt_authentication.on_jwt_created > lexik_jwt_authentication.on_authentication_success we can place all the token creation logic on the lexik_jwt_authentication. on_jwt_created event, add the refresh token to the JWT, then store the generated token in a property of the class to reuse it at the lexik_jwt_authentication.on_authentication_success event which will perform the cookie saving.

So we add a private property token and we fill the method attachRefreshTokenToJWT() like this:

private ?string $token = null;

public function attachRefreshTokenToJWT(JWTCreatedEvent $event): void
    {
        /* ↓ This code is repeated exactly as in 
             Gesdinet\JWTRefreshTokenBundle\EventListener\AttachRefreshTokenOnSuccessListener::attachRefreshToken()
        */
        $user = $event->getUser();

        if (!$user instanceof UserInterface) {
            return;
        }

        $data = $event->getData();
        $request = $this->requestStack->getCurrentRequest();
        
        if (null === $request) {
            return;
        }

        // Extract refreshToken from the request
        $refreshTokenString = $this->extractor->getRefreshToken($request, $this->tokenParameterName);

        // Remove the current refreshToken if it is single-use
        if ($refreshTokenString && true === $this->singleUse) {
            $refreshToken = $this->refreshTokenManager->get($refreshTokenString);

        /* ↓ when the route is 'gesdinet_jwt_refresh_token', we are in token renewal, not in login. 
             We can check the correspondence of the user names, and throw an exception in case of 
             mismatch (the exception we created above)
        */
            if (
                $this->httpUtils->checkRequestPath($request, 'gesdinet_jwt_refresh_token') &&
                $refreshToken->getUsername() !== $user->getUserIdentifier()
            ) {
                throw new UserNotMatchException();
            }
            $refreshTokenString = null;

        /* ↓ We continue with the copy of the original code 
        */
            if ($refreshToken instanceof RefreshTokenInterface) {
                $this->refreshTokenManager->delete($refreshToken);
            }
        }

        // Set or create the refreshTokenString
        if ($refreshTokenString) {
            $data[$this->tokenParameterName] = $refreshTokenString;
        } else {
            $refreshToken = $this->refreshTokenGenerator->createForUserWithTtl($user, $this->ttl);

            $this->refreshTokenManager->save($refreshToken);
            $refreshTokenString = $refreshToken->getRefreshToken();
            $data[$this->tokenParameterName] = $refreshTokenString;
        }

        if ($this->returnExpiration) {
            $data[$this->returnExpirationParameterName] = time() + $this->ttl;
        }

        /* ↓ We save the token for the second part
        */
        $this->token = $data[$this->tokenParameterName];

        // Set response data
        $event->setData($data);
    }

Finally, we implement the method attachRefreshToken() with the cookies part :

 public function attachRefreshToken(AuthenticationSuccessEvent $event): void
    {
        if ($this->token) {
            // Add a response cookie if enabled
            if ($this->cookieSettings['enabled']) {
                $event->getResponse()->headers->setCookie(
                    new Cookie(
                        $this->tokenParameterName,
                        $this->token,
                        time() + $this->ttl,
                        $this->cookieSettings['path'],
                        $this->cookieSettings['domain'],
                        $this->cookieSettings['secure'],
                        $this->cookieSettings['http_only'],
                        false,
                        $this->cookieSettings['same_site']
                    )
                );

                // Remove the refreshTokenString from the response body
                if (isset($this->cookieSettings['remove_token_from_body']) && $this->cookieSettings['remove_token_from_body']) {
                    unset($data[$this->tokenParameterName]);
                }
            }
        }
    }

Now, when you log in, you have only one JWT token

image

It contains your refresh token, and its expiration date (if activated in the options)

image

To renew the token, just go to api/token/refresh, with its authorization: Bearer ...... header containing a valid, even expired, JWT token. There is nothing more to put in the body.

image

And here is a brand new token!

image

AimSai59 avatar Aug 08 '22 17:08 AimSai59

If I'm being totally honest, I would not feel comfortable merging this implementation into any of my apps. This isn't to say the implementation is flawed, but IMO coupling the refresh token to a JWT and forcing the app to allow a refresh request to work with an expired token feels like a compromise is being made somewhere with security.

Other feedback:

  • If you're attaching the refresh token to a JWT, then do you really need to store the refresh tokens in your database?
  • Personally, I'm also not fond of relying on a Doctrine event listener to do the hashing. It feels like the places setting the token's value should be dealing with that; with the current implementation, you're going to have a "dirty" entity state at all times because the listener is transparently hashing then restoring a plaintext value from its own internal state.
  • Instead of using $_ENV at runtime, inject the secret using the kernel.secret container parameter into your hasher class.

mbabker avatar Aug 08 '22 18:08 mbabker

I noticed that each connection generates a new token record, while renewals are just simple updates. So when a new token is generated, in attachRefreshTokenToJWT(), I can use the request headers to add session information (user-agent in particular) that allows my users to verify that a session is legitimate. By keeping the tokens in the database, I add a layer of verification. I allow my users to revoke a session simply by deleting the token from the database. Once the offending JWT has expired, it no longer allows the session renewal.

AimSai59 avatar Aug 08 '22 18:08 AimSai59

@AimSai59

I believe what you want is an API-Gateway! 😄

Instead of the actual JWT + Refresh-Token, the FE will simply get an Access Token with expiration time:

  • access_token (string)
  • access_token_expires_at (timestamp)

Then it's the FEs job to check if access_token is still good before every request:

  • When access_token_expires_at is expired (or is about to expire):
    1. Call /api/extend (or similar) to bump the session TTL and receive an increased access_token_expires_at
    2. Make the actual request
  • Else
    1. Just make the actual request

In short:

  • BE <-> API-gateway: JWT + Refresh Token
  • FE <-> API-gateway: good ol' Session-Token (here "Access Token")
  • JWTs seem only to make sense for BE-to-BE communcation!

Advantages:

  • FE has no concept of JWTs and Refresh-Tokens
  • Users can log out! (Without having to manage a block-list of logged-out-JWTs)
    • you can even log out all users of a system at once

Disadvantages:

  • This is basically Sessions with extra steps... (read: a central bootleneck)
  • Performance overhead (every request needs to be redirected)

sebastianstucke87 avatar Aug 08 '22 19:08 sebastianstucke87

1_wMpM32WdmsUJhUhPvRyTLA

(Source: https://blog.kloia.com/using-tyk-io-and-jwt-io-on-stateless-microservice-authentication-d7e6985b97bd)

sebastianstucke87 avatar Aug 08 '22 19:08 sebastianstucke87

Sure it would be easier and more logical to use a system like this, but my API is based on api-platform which supports JWT tokens particularly well. Implementing a new authentication system on api-platform seems like a lot of work to me, while the system I described above seems functional and reliable enough.

I wanted to clarify that the /api/token/refresh entry point is the only one that allows access with a VALID but expired JWT token. In the initial process, there are NO restrictions on access to this resource anyway. So there is no compromise on security, since no other resource is accessible with an expired token.

I also replaced $_ENV['APP_SECRET'] with $container->getParameter('kernel.secret') :)

AimSai59 avatar Aug 08 '22 20:08 AimSai59