Skip to content

Conversation

@dunglas
Copy link
Member

@dunglas dunglas commented Apr 2, 2021

Generating JWT without expiration is a bad security practice. This PR adds the ability to set a default lifetime for generated tokens using the new Authorization util.

@dunglas dunglas force-pushed the feat/exp branch 3 times, most recently from e6dfd98 to a83cfa8 Compare April 2, 2021 10:30
@chalasr
Copy link
Member

chalasr commented Apr 2, 2021

👍 For the record, I think this is the only standard claim that should be set with a good default by this library.
I mean, after the recent work done on the authorization part, people will eventually start asking for iss and other claims to be set automatically, then ask for a configurable clock skew, ... endless story.
The answer should remain the same as before to me: decorate the existing token provider/factory on your own, the system is flexible and pluggable enough.

@dunglas
Copy link
Member Author

dunglas commented Apr 2, 2021

I changed how the exp claim is generated, and added a way to control the cookie expiration time accordingly:

the lifetime of the cookie, the "exp" claim of the JWT will be set accordingly, set to null to use the default value and to 0 to set a session cookie (the default expiration time of the JWT will be 1 hour)

@dunglas dunglas merged commit 49693bf into symfony:main Apr 3, 2021
@dunglas dunglas deleted the feat/exp branch April 3, 2021 09:03
azjezz pushed a commit to azjezz/mercure that referenced this pull request Jun 23, 2021
…ny#52)

* Allow to set a default expiration value on the generated token

* move the logic to the token factory

* cs

* Set cookie expiration

* more advanced logic

* cs

* fix tests

* fix tests

* fix gha

* try to fix gha

* changelog

* typo

* nico's review

* fix tests

* fix fallback

* simplify: we can modify the cookie expiration time after

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants