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

JWT Refresh Token #4371

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Timshel
Copy link
Contributor

@Timshel Timshel commented Feb 20, 2024

To facilitate review decided to move out the switch to a JWT refresh_token from the sso PR #3899.

Without the SSO logic it's not the most useful still :

  • Add an expiration on the refresh_token (work like an idle timer reset when a new access_token is generated).
  • Store the information of the AuthMethod in the token (Password ...).

@Timshel
Copy link
Contributor Author

Timshel commented Apr 10, 2024

When discussed with @BlackDex we thought that 7 days might be an appropriate value for the refresh_token expiration.

Had some feedback on it, and it might be a bit short, since many actions in the application will not trigger a refresh (reading password is done as "offline"`).

Might make sense to raise it to something around 30 days as mentioned in this documentation.

@BlackDex
Copy link
Collaborator

I agree, lets keep it the same as Bitwarden, and 7 days is a bit short indeed.
Also, how is this linked to the refresh roll PR? Could that potentially break offline logins too?

@Timshel
Copy link
Contributor Author

Timshel commented Apr 10, 2024

Also, how is this linked to the refresh roll PR?

This is a superset of the refresh roll PR, the same logic of changing the device token is used to prevent reusing twice the same refresh_token.

Could that potentially break offline logins too?

I'm unsure what are the offline logins ?

@BlackDex
Copy link
Collaborator

Also, how is this linked to the refresh roll PR?

This is a superset of the refresh roll PR, the same logic of changing the device token is used to prevent reusing twice the same refresh_token.

Could that potentially break offline logins too?

I'm unsure what are the offline logins ?

I mean offline unlock in some way. But i doubt it.

@Timshel
Copy link
Contributor Author

Timshel commented Apr 10, 2024

A yes when unlocking I don't believe any call is done to the refresh endpoint, so there should be no issue/change.

@Timshel Timshel force-pushed the feature/jwt_refresh branch 2 times, most recently from f015cc2 to 3b77eb3 Compare April 15, 2024 14:36
@Timshel
Copy link
Contributor Author

Timshel commented Apr 15, 2024

Removed the rolling of the device token since it caused issues when the web client called the refresh token endpoint in parallel.

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.

None yet

2 participants