search

gabesullice / jwt

https://www.drupal.org/project/jwt
18 stars 12 forks source link

Add refresh module and a bit of cleanup. #11

Closed bradjones1 closed 7 years ago

bradjones1 commented 7 years ago

Adds basic support for refresh token storage. I think a new content entity type is required here since we need some storage mechanism for the refresh tokens in order to support revocation. Thoughts?

bradjones1 commented 7 years ago

Quick edit forthcoming to handle basic user status (enabled) and flood control akin to the login controller.

https://github.com/gesdinet/JWTRefreshTokenBundle seems to be similar and stores the refresh tokens in plaintext, which seems like it would be required for lookup and validation. They're opaque/random so not linked to any user input, but ideas about best practices on storage would be appreciated.

jonathangreen commented 7 years ago

Thanks so much for the contribution @bradjones1!

I'm not sure what the best practices for a storing refresh tokens would be. Maybe could hook into the Key module and use its storage mechanisms, but I'm not sure if thats really a use case they support. @gabesullice might have some more ideas for you.

bradjones1 commented 7 years ago

@gabesullice Great thinking on the jti key. I'll refactor.

bradjones1 commented 7 years ago

@gabesullice Thanks for the feedback; still lacks tests (but so does the whole module...) but here's a much cleaner approach. Also supports expiration of refresh tokens and cleanup of expired tokens on cron.

bradjones1 commented 7 years ago

@gabesullice I reworked this to use the uuid for the jti value; I'm about at the end of my time on this re: the project I'm currently working on at the moment, so what do you think of merging this at this point and then we can talk about a broader refactor of the token generation? I get what you're saying on the custom entity storage but I think loadByProperties() should work for the moment, as we're doing now. Thoughts?