fix: Check auth validity before setting isAuthenticated

[julian-determined-ai]

Description

There is bug in GenAI currently where a user gets stuck in an infinite redirect loop, this fixes the issue.

You can see the bug in action below:
https://github.com/determined-ai/determined/assets/103522725/7375fff4-bdc6-4c78-a6f0-5717080cf21f

The ultimate cause of the bug is that if you look at the logic in the sign in page we:

1. Check if the user `isAuthenticated"
2. Check if there is a redirect, if there is we redirect the user.

However the issue is that in (1) the isAuthenticated can be set to true without the user actually being a valid and authenticated user. This causes an infinite login bug in GenAI since we use the MLDE login page for authentication also. The reason that this is not an issue in MLDE is because after sign-in:

1. The user is redirected to the dashboard page
2. As soon as an API call fails, we catch the auth error, and remove the bad token
3. We instantly send the user back to the sign-in page.

However (2) cannot happen in GenAI since the invalid token lives and is controlled on the MLDE app.

In order to preserve all current log in and auth related behavior, this PR introduces an extra check to ensure the user is authenticated if an auth-token is present. If not, then we handle the error using out handleError function. This set up ensures that unauthenticated users will not be improperly redirected.

Test Plan

1. Ensure that you are logged out of MLDE.
2. Navigate to the login page.
3. Using devtools set a value for the global/auth-token
4. Add a ?redirect query to the login url such as login?redirect=/lore/
5. Visit the URL in (4).
6. Ensure that you are not redirected, and you instead immediately shown a "loading" message and then the login page again.

Commentary (optional)

Checklist

• Changes have been manually QA'd
• User-facing API changes need the "User-facing API Change" label.
• Release notes should be added as a separate file under docs/release-notes/.
  See Release Note for details.
• Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

[netlify]

✅ Deploy Preview for determined-ui ready!

Name	Link
🔨 Latest commit	a409040
🔍 Latest deploy log	https://app.netlify.com/sites/determined-ui/deploys/65e8ee5fc4e216000869fc9f
😎 Deploy Preview	https://deploy-preview-8967--determined-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov]

Codecov Report

Attention: Patch coverage is 16.66667% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 42.60%. Comparing base (191a144) to head (a409040).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8967      +/-   ##
==========================================
- Coverage   47.37%   42.60%   -4.77%     
==========================================
  Files        1162      842     -320     
  Lines      176134   136791   -39343     
  Branches     2237     2236       -1     
==========================================
- Hits        83448    58286   -25162     
+ Misses      92528    78347   -14181     
  Partials      158      158              

Flag	Coverage Δ
harness	?
web	42.54% <16.66%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
webui/react/src/hooks/useAuthCheck.ts	37.34% <16.66%> (-1.12%)	⬇️

... and 320 files with indirect coverage changes

[hkang1]

Thanks for addressing this!

Had some questions on the changes.

[hkang1]

nit: typo properly

[hkang1]

question: if it fails, should the source of the token get cleared? Things like:

• jwt token from searchParams
• cookieToken not sure if anything can or needs to be done here
• globalStorage.authToken clear this?

[julian-determined-ai]

Calling handleError ended up being the best approach here. I ran into unwanted beahvior/bugs otherwise. I am happy to delve into it though if need be.

The handleError method eventually calls paths.logout which properly entirely resets the application and removes the tokens.

Regarding removing the jwt from the search params. I am not sure that we want to remove that here, since we have logic that explicitly checks for it and I am not sure if removing it will cause unwanted side effects. But if we think it is okay to remove here then I am open to it.

[johnkim-det]

LGTM