Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reuse_access_token config should be used when refreshing a token #1663

Open
PhilippeChab opened this issue Jul 10, 2023 · 4 comments
Open

reuse_access_token config should be used when refreshing a token #1663

PhilippeChab opened this issue Jul 10, 2023 · 4 comments

Comments

@PhilippeChab
Copy link

Maybe I am wrong, but I feel like reuse_access_token should apply when refreshing a token that has not expired yet.

An improper use of the refesh_token grant type could lead to the creation of an unnecessary number of access tokens and bloat the underlying database.

I believe the only change needed to make this possible would be to replace create_for here by the find_or_create_for method of the access token mixin.

@ThisIsMissEm
Copy link

I'd disagree with this, as if you are using refresh tokens with reuse_access_token, you're explicitly saying "please generate a new access token", i.e., I think what you're proposing would render refresh tokens pointless.

@PhilippeChab
Copy link
Author

PhilippeChab commented Nov 20, 2023

I'd disagree with this, as if you are using refresh tokens with reuse_access_token, you're explicitly saying "please generate a new access token", i.e., I think what you're proposing would render refresh tokens pointless.

Well, it is definitively not pointless, since you'll need to refresh your access token eventually nonetheless.

But I understand that if you explicitly ask to refresh your current token, it could be considered weird/a bug to return the current, unexpired token. If it is a matter of security, the "good" way to invalidate your tokens would be to revoke them however.

I don't know. Maybe it should be configurable?

@ThisIsMissEm
Copy link

ThisIsMissEm commented Nov 20, 2023

I'd probably say "reuse_access_token" should maybe be considered incompatible with refresh tokens? Maybe?

Token reuse in general is a really bad idea, imo, and not having expiry on tokens that are not PATs is really not a good idea, but even PATs should probably expire & be refreshed regularly.

@PhilippeChab
Copy link
Author

@nbulaj I am curious, what is your opinion on this?

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

No branches or pull requests

2 participants