Skip to content

UI token refreshing#3842

Merged
foot merged 5 commits intomainfrom
ui-token-refreshing
Aug 8, 2023
Merged

UI token refreshing#3842
foot merged 5 commits intomainfrom
ui-token-refreshing

Conversation

@foot
Copy link
Copy Markdown
Contributor

@foot foot commented Jul 13, 2023

Closes #3832

What changed?

Token refreshing is now handled by the UI, not in-request by the server.

Why was this change made?

We cannot lock the refresh process on the server well, new requests will still come in and re-trigger it etc.

How was this change implemented?

Add more logic the UI to deal with expired tokens

  • Extend the UnAuthorizedInterceptor to try and refresh the token if we get a 401
  • Keep track of inflight tokenRefreshes so we only have a single one at a time
  • Wait for token refresh to complete before retrying requests

On the server we remove the inline token refresh logic

How did you validate the change?

Manual testing mainly:

  1. Setup up weave-gitops + OIDC w/ a provider that supports offline-access
  2. Login
  3. delete id_token cookie
  4. Watch 401s return and a single refresh request made that restores the id_token
kind create cluster
kubectl create secret generic oidc-auth \
  --namespace default \
  --from-literal=issuerURL=<your-issuer-url> \
  --from-literal=clientID=<your-client-id> \
  --from-literal=clientSecret=<your-oidc-client-secret> \
  --from-literal=redirectURL=http://localhost:4567/oauth2/callback
FAST_AND_FURIOUSER=1 SKIP_UI_BUILD=1 tilt up

npm install
npm start
open http://localhost:4567

Waterfall

Note 401s, single refresh, immediately followed by the 200s

image

Follow ups:

  • The refresh token navigates away, but resolves the promise, it perhaps should not do that, so no new requests start.
  • /userinfo should also be wrapped by the unauth'dwrapper the UI too, its the first endpoint we hit when loading the UI to see if the user is logged in. We don't do this yet, so if you have a refresh-token and navigate to the UI fresh, we don't try and get you a new token right now.

@foot foot force-pushed the ui-token-refreshing branch from e1ed14b to 45a8125 Compare July 20, 2023 11:39
@foot foot marked this pull request as ready for review July 20, 2023 12:17
@foot foot changed the title [WIP] UI token refreshing UI token refreshing Jul 21, 2023
- Extend the UnAuthorizedInterceptor to try and refresh the token if we
  get a 401
- Keep track of inflight tokenRefreshes so we only have a single one at
  a time
- Wait for token refresh to complete before retrying requests
- Remove the backend retry logic
@foot foot force-pushed the ui-token-refreshing branch from 0244ef8 to 2088e46 Compare July 21, 2023 04:42
@foot foot requested review from AlinaGoaga and bigkevmcd August 7, 2023 12:52
Copy link
Copy Markdown
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

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

This looks great!

Copy link
Copy Markdown
Contributor

@AlinaGoaga AlinaGoaga left a comment

Choose a reason for hiding this comment

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

Looks great, tested locally and it's working as expected!

- Proxy is a bit obscure API, it was originally used in the hope that it
  would be able to provide better type safety, but I couldn't figure out
  how to get it there.
- Revert to code more similar to the original implementation which just
  creates a returns new simple object.
@foot foot merged commit 0f813db into main Aug 8, 2023
@foot foot deleted the ui-token-refreshing branch August 8, 2023 13:43
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.

OIDC Refresh token renew twice and 401 Re-auth is occur after id_token expiration since 0.22

3 participants