Skip to content

feat: add DISABLE_REFRESH_TOKEN_NBF to disable refresh_token.nbf#653

Merged
sebadob merged 3 commits intomainfrom
651-opt-out-refresh-token-nbf
Dec 13, 2024
Merged

feat: add DISABLE_REFRESH_TOKEN_NBF to disable refresh_token.nbf#653
sebadob merged 3 commits intomainfrom
651-opt-out-refresh-token-nbf

Conversation

@sebadob
Copy link
Owner

@sebadob sebadob commented Dec 13, 2024

fixes #651

@sebadob sebadob merged commit 2aecf6e into main Dec 13, 2024
@sebadob sebadob deleted the 651-opt-out-refresh-token-nbf branch December 13, 2024 10:30
Copy link
Contributor

@andoks andoks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope you don't mind, I had a look in liù of not being able to test it. Only thing I could find was some potential "typos" in the documentation.

Comment on lines +19 to +21
# However, there are bad / lazy client implementations that do not respect
# either `nbf` in the `refresh_token`, or the `exp` claim in `access_token`
# and will refresh early while the current access_token is still valid.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the impression that refresh_tokens usually are opaque (which seems to be supported by at least one of the standards), so I am not sure nbf-checking of refresh-tokens usually would be related to a projects quality 😅

AT expiry though, should probably best be checked (at least for clients like oauth2-proxy)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. exp checking on the access_token must be done though and I don't see any good reason why you would refresh a still valid access_token, other than simply being lazy in the implementation.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the refresh token just another JWT to give clients the possibility to inspect it and gain further information. Even though it's non-standard, it's an additional feature.

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.

Unable to redeem refresh_token

2 participants