-
Notifications
You must be signed in to change notification settings - Fork 39
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
refresh token functionality #40
Comments
If someone submits a PR, sure. |
studer-l
pushed a commit
to studer-l/ring-oauth2
that referenced
this issue
Jan 10, 2025
Adds an additional flow to the existing middleware to refresh expired tokens. The implementation handles multiple active grants. Expired grants that failed to refresh are removed from the `:session`. The refresh workflow is unconditionally activated. For simplicity, the access tokens are now always added to the response's session. If a refresh occurs, the tokens in `:session` are left as-is, the updated access tokens are accessibly via the existing `:oauth2/access-token` key. This allows downstream handlers to observe that a token refresh occurred. There is a potential bug, where concurrent requests with expired tokens may race. For example, consider a page containing a `css` and a `js` resource. If a user's access token was to expire exactly as the `index.html` finishes loading, their browser may concurrently fetch both the `css` and `js` resources, one of which will then fail. I do not see a way to address this without introducing considerable complexity. I have added a timeout of 60 seconds to the refresh http request, which means a slow oauth backend will cause users to become logged out. I think this is more informative for users than hanging forever. Fixes weavejester#40
studer-l
pushed a commit
to studer-l/ring-oauth2
that referenced
this issue
Jan 10, 2025
Adds an additional flow to the existing middleware to refresh expired tokens. The implementation handles multiple active grants. Expired grants that failed to refresh are removed from the `:session`. The refresh workflow is unconditionally activated. For simplicity, the access tokens are now always added to the response's session. If a refresh occurs, the tokens in `:session` are left as-is, the updated access tokens are accessibly via the existing `:oauth2/access-token` key. This allows downstream handlers to observe that a token refresh occurred. There is a potential bug, where concurrent requests with expired tokens may race. For example, consider a page containing a `css` and a `js` resource. If a user's access token was to expire exactly as the `index.html` finishes loading, their browser may concurrently fetch both the `css` and `js` resources, one of which will then fail. I do not see a way to address this without introducing considerable complexity. I have added a timeout of 60 seconds to the refresh http request, which means a slow oauth backend will cause users to become logged out. I think this is more informative for users than hanging forever. Fixes weavejester#40
studer-l
pushed a commit
to studer-l/ring-oauth2
that referenced
this issue
Jan 10, 2025
Adds an additional flow to the existing middleware to refresh expired tokens. The implementation handles multiple active grants. Expired grants that failed to refresh are removed from the `:session`. The refresh workflow is unconditionally activated. Since a token refresh may occur for any request, access tokens are now always added to the response's `:session`. This may break code which previously relied on the `:session` only being set during the initial grant workflow. I do not think this can be avoided. If a refresh occurs, the tokens in `(:session request)` are left as-is, the updated access tokens are accessibly via the existing `:oauth2/access-token` key. This allows downstream handlers to observe that a token refresh occurred. There is a potential bug, where concurrent requests with expired tokens may race. For example, consider a page containing a `css` and a `js` resource. If a user's access token was to expire exactly as the `index.html` finishes loading, their browser may concurrently fetch both the `css` and `js` resources, triggering two concurrent token refresh attempts with the same token, one of which may fail. I do not see a way to address this without introducing considerable complexity. I have added a timeout of 60 seconds to the refresh http request, which means a slow oauth backend will cause users to become logged out. I think this is more informative for users than hanging forever. Fixes weavejester#40
studer-l
pushed a commit
to studer-l/ring-oauth2
that referenced
this issue
Jan 10, 2025
Adds an additional flow to the existing middleware to refresh expired tokens. The implementation handles multiple active grants. Expired grants that failed to refresh are removed. The refresh workflow is unconditionally activated. Since a token refresh may occur for any request, access tokens are now always added to the response's `:session`. This may break code which previously relied on the `:session` only being set during the initial grant workflow. I do not think this can be avoided. If a refresh occurs, the tokens in `(:session request)` are left as-is, the updated access tokens are accessibly via the existing `:oauth2/access-token` key. This allows downstream handlers to observe that a token refresh occurred. There is a potential bug, where concurrent requests with expired tokens may race. For example, consider a page containing a `css` and a `js` resource. If a user's access token was to expire exactly as the `index.html` finishes loading, their browser may concurrently fetch both the `css` and `js` resources, triggering two concurrent token refresh attempts with the same token, one of which may fail. I do not see a way to address this without introducing considerable complexity. I have added a timeout of 60 seconds to the refresh http request, which means a slow oauth backend will cause users to become logged out. I think this is more informative for users than hanging forever. Fixes weavejester#40
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Ever consider implementing that? It's a pretty normal thing for Oauth2 right?
The text was updated successfully, but these errors were encountered: