markitosgv / JWTRefreshTokenBundle

Implements a Refresh Token system over Json Web Tokens in Symfony
MIT License
663 stars 159 forks source link

Use another table for refresh tokens #285

Closed sukhoy94 closed 2 years ago

sukhoy94 commented 2 years ago

Is it possible to use another table name to store and check refresh tokens instead of public.refresh_tokens ?

mbabker commented 2 years ago

Using the 1.0 betas, follow https://github.com/markitosgv/JWTRefreshTokenBundle/blob/master/README.md#use-another-entity-for-refresh-tokens to use your own entity class, and you can take ownership of the database schema with that.

sukhoy94 commented 2 years ago

Using the 1.0 betas, follow https://github.com/markitosgv/JWTRefreshTokenBundle/blob/master/README.md#use-another-entity-for-refresh-tokens to use your own entity class, and you can take ownership of the database schema with that.

Thanks, I will try it :)

sukhoy94 commented 2 years ago

@mbabker

Got some strange issue, app crashes on this sql:

SELECT t1.refresh_token AS refresh_token_2, t1.username AS username_3, t1.valid AS valid_4, t1.id AS id_5 FROM jwt_refresh_token t1 WHERE t0.refresh_token = ? LIMIT 1

btw. why it uses t0 alias instead of t1 in where statement ?

mbabker commented 2 years ago

No idea, that'd be Doctrine computing the table aliases.

sukhoy94 commented 2 years ago

Ok, works for me in some other variation. First of all my refresh token class must extends not Gesdinet\JWTRefreshTokenBundle\Entity but Gesdinet\JWTRefreshTokenBundle\Model\AbstractRefreshToken, and i should redefine all fields in my entity.

Full code is below, now my tokens are stored in custom_schema.refresh_tokens table.

@markitosgv can you please confirm that this is right way for Symfony 5.3* ?


use Doctrine\ORM\Mapping as ORM;
use Doctrine\ORM\Mapping\Table;
use Doctrine\ORM\Mapping\UniqueConstraint;
use Gesdinet\JWTRefreshTokenBundle\Model\AbstractRefreshToken;

#[ORM\Entity(repositoryClass: \Gesdinet\JWTRefreshTokenBundle\Entity\RefreshTokenRepository::class)]
#[Table(name: "refresh_tokens", schema: "custom_schema")]
#[UniqueConstraint(name: "jwt_auth_refresh_tokens_unique", columns: ["refresh_token"])]
class RefreshToken extends AbstractRefreshToken
{
    #[ORM\Id]
    #[ORM\Column(name: 'id', type: 'integer', nullable: false)]
    #[ORM\GeneratedValue(strategy: 'AUTO')]
    protected $id;

    #[ORM\Column(name: 'refresh_token', type: 'string', nullable: false)]
    protected $refreshToken;

    #[ORM\Column(name: 'username', type: 'string', nullable: false)]
    protected $username;

    #[ORM\Column(name: 'valid', type: 'datetime', nullable: false)]
    protected $valid;

    /**
     * {@inheritdoc}
     */
    public function getId()
    {
        return $this->id;
    }
}
mbabker commented 2 years ago

I'm not sure why you can't just extend the entity and why you have to extend the model to make it work (before I wrote that part of the readme, I did test customizing the entity in an application by extending the entity class and things just worked without any hiccup), but glad that's working for you at least.

sukhoy94 commented 2 years ago

I'm not sure why you can't just extend the entity and why you have to extend the model to make it work (before I wrote that part of the readme, I did test customizing the entity in an application by extending the entity class and things just worked without any hiccup), but glad that's working for you at least.

I think I will test it (that code example from documentation) one more time on fresh Symfony 5.3* in weekend, and if it won't works, I'll create some PR :)

valkars commented 2 years ago

Hello, when I use simple extend (Entity\RefreshToken or Model\AbstractRefreshToken) and empty class, I receive an error:

No identifier/primary key specified for Entity "App\Entity\JwtRefreshToken" sub class of "Gesdinet\JWTRefreshTokenBundle\Entity\RefreshToken". Every Entity must have an identifier/primary key. class JwtRefreshToken extends \Gesdinet\JWTRefreshTokenBundle\Entity\RefreshToken { }

piciuok commented 2 years ago

I have a same problem, i cannot use custom table name: when extending like shown in readme i got two tables: image

I'am using 1.0.0-beta4 version with symfony 5.4

john-dufrene-dev commented 2 years ago

Same error when I extending, 2 entities were created,

Anyone have an idea ?

mbabker commented 2 years ago

I think I figured it out, but there isn't a great fix that doesn't require folks to do stuff in their apps when the bundle code is fixed (and a Flex recipe should probably be submitted for 1.0 to automate this as much as practical).

Essentially, the provided document and entity classes need to have their mappings updated to be set as mapped superclasses instead of concrete objects. In doing that, downstream users will have to add their own document/entity class in their applications and set the refresh_token_class config parameter appropriately (i.e. App\Entity\RefreshToken instead of the default Gesdinet\JWTRefreshTokenBundle\Entity\RefreshToken). For anyone who has ever used the FOSOAuthServerBundle, what would be needed here is similar to what you'd need to do when setting up that bundle for the first time.

I don't know when exactly I broke this, but I'm pretty sure one of the many refactorings I've done for this bundle managed to break it along the way. Sorry about that.

mbabker commented 2 years ago

Now that I've gotten the Symfony 6 thing out of the way, the below changes are my suggested way forward (based on what's in my last comment):

If anyone's got any feedback on these changes, or would like to test them before PRs are sent up (you'll need to manually patch your application for now, but it's all pretty straightforward with a 2-line change in an XML file and a 1-line change in a YAML file (3 if you have to create the file)), that would help with getting this issue closed out.

mbabker commented 2 years ago

@markitosgv Any feedback on the above comments? This one would've been good to fix before tagging a 1.0 stable since the mapping change would be considered a B/C break.

valkars commented 2 years ago

I have auto_mapping: false, so I added mapping for GesdinetJWTRefreshTokenBundle and this patch works with empty entity:

use Doctrine\ORM\Mapping as ORM;
use Gesdinet\JWTRefreshTokenBundle\Entity\RefreshToken;

/**
 * @ORM\Table("my_table")
 */
class JwtRefreshToken extends RefreshToken
{
}
mbabker commented 2 years ago

See https://github.com/markitosgv/JWTRefreshTokenBundle/pull/304 for the PR with my proposed patch above.