Skip to content

Conversation

@iOvergaard
Copy link
Contributor

@iOvergaard iOvergaard commented Jul 2, 2025

Description

This is a follow-up from #19495

If a token response is rejected, then the pipeline will also fail because it does not understand that error. Let the API interceptors do their job instead and simply return the old, now-invalid token, which will prompt the API interceptors to store the request states and retry them afterwards.

Details

The error is prompted because the /token endpoint on the server returns a 400 error, which the client doesn't understand. Instead of relying on server errors, we merely assume that no new token was received. This means that the following requests will be stored and retried once a real login happens.
(The login is prompted automatically by the API interceptors, so the AuthFlow does not need to call timeout itself)

How to test

  1. Set a relatively low TimeOut in appsettings.json on Umbraco:CMS:Global:
"TimeOut": "00:00:00:10"
  1. Log in and wait 10 seconds, then perform an action
  2. Verify that the content is refreshed after log in.

Before

TimeoutMedia

After

2025-07-02.at.13.54.08.-.Red.Flamingo.mp4

Copilot

This pull request modifies the performWithFreshTokens method in auth-flow.ts to improve token refresh handling and clarify its behavior. The key changes include updating the return type documentation, adding logic to clear token state when the refresh fails, and simplifying the method's structure.

Improvements to token refresh handling:

  • src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts: Updated the return type documentation of performWithFreshTokens to specify {Promise<string>}. Added logic to clear token storage when the refresh token request fails and removed redundant code for handling missing tokenResponse. Simplified the method to ensure a valid access token is returned or an empty string if unavailable.

If a token response is rejected, then the pipeline will also fail because it does not understand that error. Let the API interceptors do their job instead and simply return the old, now-invalid token which will prompt the API interceptors to store the request states and retry them afterwards.
Copilot AI review requested due to automatic review settings July 2, 2025 11:49
Copy link
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 updates the token refresh flow so that it never rejects on failure, letting API interceptors handle retries. Key changes include:

  • Updated JSDoc return annotation to Promise<string>
  • Removed rejection and #timeoutSignal on refresh failure
  • Always resolves with (possibly empty) token string and clears stored tokens on failure
Comments suppressed due to low confidence (2)

src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts:308

  • Update the JSDoc to document the failure path: mention that on refresh failure this method will clear stored tokens and return an empty string (or the stale token if you choose to preserve it).
	 * @returns {Promise<string>} The access token for the user.

src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts:325

  • Add unit tests for the failure scenario to verify that clearing storage and returning an empty (or stale) token behaves as expected and triggers your API interceptors.
		return Promise.resolve(this.#tokenResponse?.accessToken ?? '');

@andr317c
Copy link
Contributor

andr317c commented Jul 2, 2025

LGTM! It works now on my machine 😄

Copy link
Member

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

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

Tested out, works as advertised. 🚀

@leekelleher leekelleher merged commit c9bef96 into release/16.1 Jul 2, 2025
23 checks passed
@leekelleher leekelleher deleted the v16/bugfix/token-timeout-400-errors branch July 2, 2025 15:10
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.

4 participants