Skip to content

Auth: Skip /token refresh when access token is still valid#22087

Merged
leekelleher merged 6 commits intomainfrom
v17/improvement/skip-token-refresh-when-access-token-valid
Mar 11, 2026
Merged

Auth: Skip /token refresh when access token is still valid#22087
leekelleher merged 6 commits intomainfrom
v17/improvement/skip-token-refresh-when-access-token-valid

Conversation

@iOvergaard
Copy link
Copy Markdown
Contributor

@iOvergaard iOvergaard commented Mar 11, 2026

Summary

  • Skip /token when access token is still validconfigureClient()'s auth callback and getLatestToken() now guard with #isAccessTokenValid(), so the token endpoint is only hit when the access token has actually expired. Previously every API request triggered a refresh regardless, causing unnecessary token churn and OpenIddict ID2019 errors on in-flight requests.

  • Wait for ongoing cross-tab refresh — when the access token is valid but another tab holds the umb:token-refresh Web Lock (proactive refresh via keepUserLoggedIn), the auth callback now waits for that refresh to complete before proceeding. This restores the cross-tab serialization that the guard above removed, preventing requests from being sent with a token that is about to be revoked.

  • Remove redundant first-check validateToken() on startupUmbAppAuthController.isAuthorized() previously called validateToken() on the very first guard evaluation, producing a second /token call immediately after setInitialState() had already verified the session. This was a leftover from the AppAuth/localStorage era. setInitialState() now owns server verification; stale peer-sourced sessions are handled lazily by the 401 interceptor.

How each caller behaves after these changes

Caller Behaviour
configureClient() auth callback (per request) Skips /token when access token valid; waits if another tab is refreshing
getLatestToken() (deprecated, userland) Same as above
UmbAuthSessionTimeoutControllervalidateToken() Unchanged — always calls makeRefreshTokenRequest() for proactive refresh
app-auth.controller.ts startup guard Removed redundant first-check; setInitialState() is authoritative

Why the popup re-auth path doesn't call /token from the main window

After timeout → popup re-auth, the popup's authorization_code exchange calls /token and the server's HideBackOfficeTokensHandler writes fresh encrypted cookies to the response. Cookies are domain-scoped, so the main window has the new cookies immediately — no separate /token call needed. The "Stay logged in" button takes a different path (refresh_token grant directly from the main window), but both paths end with equivalent fresh cookie state.

Test plan

  • Navigate between sections — /token should only fire when the access token expires, not on every request
  • With keepUserLoggedIn=true: open two tabs, verify only one /token call fires per proactive refresh cycle (no ID2019 on the other tab)
  • Let the session timeout naturally → re-auth via popup → verify the encrypted cookie value changes and subsequent requests succeed without errors
  • Click "Stay logged in" in the timeout modal → verify /token is called and the session timer resets
  • Verify startup has exactly one /token call (not two)

🤖 Generated with Claude Code

Guard the per-request validateToken() call sites with #isAccessTokenValid()
in configureClient() and getLatestToken(). Previously, every API request
triggered a /token call even when the access token had not expired, causing
unnecessary token churn and OpenIddict ID2019 errors for in-flight requests.

Proactive refresh via UmbAuthSessionTimeoutController and startup validation
in app-auth.controller.ts are unaffected — those call validateToken() directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 11, 2026 06:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces unnecessary /token refresh calls in the backoffice client by avoiding validateToken() refreshes on every API request when the in-memory access token is still valid, aiming to reduce token endpoint load and mitigate OpenIddict ID2019 errors caused by frequent token revocation.

Changes:

  • Guard getLatestToken() to call validateToken() only when #isAccessTokenValid() is false.
  • Guard the OpenAPI client auth callback in configureClient() similarly, so requests don’t trigger refresh while the access token is still valid.

iOvergaard and others added 3 commits March 11, 2026 08:09
setInitialState() already handles server verification before the router
evaluates guards — either via a direct /token call (makeRefreshTokenRequest)
or via peer session adoption (BroadcastChannel). The #isFirstCheck guard in
UmbAppAuthController was a leftover from the AppAuth/localStorage era, where
token state was restored from storage and needed a server round-trip to confirm
validity. That assumption no longer holds: if getIsAuthorized() is true after
setInitialState(), the session came directly from the server or from a peer
whose timing is still valid. Stale/revoked peer sessions are handled lazily
by the 401 interceptor, which triggers re-auth as needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Restores the cross-tab lock serialization that was implicitly provided by
the old unconditional validateToken() call. When another tab holds the
umb:token-refresh lock (keepUserLoggedIn proactive refresh), API requests
in this tab now wait for it to complete before proceeding. This prevents
sending requests with an access token that is about to be revoked, which
caused OpenIddict ID2019 errors on in-flight requests.

The fast path (token valid, no refresh in progress) remains: navigator.locks.query()
is a cheap browser-internal call, and the lock.request() no-op is only
incurred when a cross-tab refresh is actually happening.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Extract duplicate guard logic from configureClient() and getLatestToken()
  into a single #ensureTokenReady() private method
- Rename from #ensureValidToken() → #ensureTokenReady() to distinguish from
  the validate/valid naming cluster (validateToken, isAccessTokenValid)
- Add JSDoc to #isAccessTokenValid() clarifying it is a local timestamp check
  with no network call
- Improve JSDoc on validateToken() to make clear it forces a network refresh
  (unconditional /token call), distinct from the per-request #ensureTokenReady()
  gate which skips the call when the access token is still live

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@leekelleher leekelleher self-requested a review March 11, 2026 08:31
iOvergaard and others added 2 commits March 11, 2026 09:45
…nously inside lock

With keepUserLoggedIn=true and a short access token lifetime (e.g. expiresIn ≤ buffer),
#updateSession() triggers session$ synchronously inside the lock callback. The observer
fires #scheduleCheck → #onSessionExpiring → validateToken() before the lock is released.
This re-entrant call captures sessionBefore = newSession (already updated), so the
reference guard cannot detect it, resulting in a duplicate /token request.

Fix by tracking #inSessionUpdateCallback around the #updateSession() call. Re-entrant
callers return true immediately; concurrent non-re-entrant callers are unaffected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants