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

Roll the refresh_token #4364

Closed
wants to merge 2 commits into from

Conversation

Timshel
Copy link
Contributor

@Timshel Timshel commented Feb 18, 2024

Hey,

We recently discussed the refresh_token in the SSO PR and during this discussion it occurred to me that the long lived refresh_token could be rolled as is done by some SSO client.

This is handled / expected by the Bitwarden clients since the response _refresh_login contain the potentially updated refresh_token.

@Timshel Timshel force-pushed the feature/rolling_refresh branch from cc62a32 to 1933304 Compare February 20, 2024 21:06
@BlackDex
Copy link
Collaborator

@Timshel, regarding the Rolling you just pushed.
Have you tried it in the web-vault?

You probably need to set the token/refresh validity lower, else you have to wait a long time hehe.

But i think it will break. It would be great of ot will not, but i have my doubts.

@Timshel
Copy link
Contributor Author

Timshel commented Feb 20, 2024

Yep, trick I found to trigger the refresh token without waiting are :

  • In web create a collection, it will trigger multiple refresh_token
  • In mobile it's more complicated, need to lower the access_token validity (like 6min) then a Synchronisation will trigger a refresh if the access_token is close to expiration (5min, so you just need to wait one minute).

@Timshel Timshel force-pushed the feature/rolling_refresh branch from 1933304 to de76218 Compare March 8, 2024 16:45
@Timshel
Copy link
Contributor Author

Timshel commented Mar 12, 2024

Any feedback ?

@BlackDex
Copy link
Collaborator

I have not had any time yet to take a good look at it.

@Timshel Timshel force-pushed the feature/rolling_refresh branch from de76218 to fa3da1b Compare March 20, 2024 15:16
@Timshel
Copy link
Contributor Author

Timshel commented Apr 15, 2024

Had issues with this, too many cases where the client call the refresh token endpoint in parallel.
So rolled it back out for now.

@Timshel Timshel closed this Apr 15, 2024
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.

2 participants