I was wondering why refresh tokens are stored clear on the database. Refresh tokens should be considered as sensitive as passwords as they can lead to full accounts compromise, and we all know we should not store passwords clear.
In this context, we do not need to use a strong password hashing such as bcrypt because refresh tokens have a very strong entropy (they are currently generated using 64 random bytes), so using SHA256 looks fair enough, and allows to find() existing tokens on the database without prior knowledge of the requester's username.
For those interested, it is already possible to hash refresh tokens, but at the cost of performance because it involves a doctrine subscriber. It may be more performant by doing it natively on this bundle; I may develop it if core contributors of this bundle think it worth it.
Implementation as of today
First, create a hash tool:
<?php
namespace App\Tools;
class Hash
{
static public function hash(string $secret) : string
{
return hash('sha256', $_ENV['APP_SECRET'].$secret);
}
}
Then, create a service that will overwrite the get() method that seeks for a refresh token on the database
<?php
namespace App\Service;
use App\Tools\Hash;
use Gesdinet\JWTRefreshTokenBundle\Doctrine\RefreshTokenManager as BaseRefreshTokenManager;
class RefreshTokenManager extends BaseRefreshTokenManager
{
public function get($refreshToken)
{
return parent::get(
Hash::hash($refreshToken)
);
}
}
<?php
namespace App\EventSubscriber\Doctrine;
use App\Tools\Hash;
use Doctrine\Bundle\DoctrineBundle\EventSubscriber\EventSubscriberInterface;
use Doctrine\ORM\Event\LifecycleEventArgs;
use Doctrine\ORM\Events;
use Gesdinet\JWTRefreshTokenBundle\Entity\RefreshToken;
class RefreshTokenSubscriber implements EventSubscriberInterface
{
private array $cleartexts = [];
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(Hash::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)]);
}
}
Finally, enforce single use of refresh tokens (because for multiple uses, we would need to read the clear token - but multiple uses of a refresh token is not a good practice anyway).
Hello mates,
I was wondering why refresh tokens are stored clear on the database. Refresh tokens should be considered as sensitive as passwords as they can lead to full accounts compromise, and we all know we should not store passwords clear.
In this context, we do not need to use a strong password hashing such as bcrypt because refresh tokens have a very strong entropy (they are currently generated using 64 random bytes), so using SHA256 looks fair enough, and allows to
find()
existing tokens on the database without prior knowledge of the requester's username.For those interested, it is already possible to hash refresh tokens, but at the cost of performance because it involves a doctrine subscriber. It may be more performant by doing it natively on this bundle; I may develop it if core contributors of this bundle think it worth it.
Implementation as of today
First, create a hash tool:
Then, create a service that will overwrite the
get()
method that seeks for a refresh token on the databaseAnd its DI to overwrite the original one:
Now, create a Doctrine subscriber:
Finally, enforce single use of refresh tokens (because for multiple uses, we would need to read the clear token - but multiple uses of a refresh token is not a good practice anyway).