Page MenuHomePhabricator

Dashboard shows expired credentials as valid
Open, In Progress, Needs TriagePublicBUG REPORT

Description

image.png (1×3 px, 889 KB)

When loading the dashboard, instead of being redirected to the login form, it shows cached data, and a "Sign out" button.

Updated desc:
A stale window can stay "logged in" without a refresh and user wouldn't know.
Hitting Cognito's /oauth2/userInfo endpoint at an interval like ~60sec and if response.status === 401 redirect to /. At that point we can display a "Your session has expired. Please log in" 'modal' box (not a modal but the alert box I forget the name of).

Acceptance criteria
The dashboard should regularly poll if session is still valid, if expired then redirect user to root (the login form).

ToDo

  • Detect expired credentials and redirect.

Event Timeline

Decide user experience first. it requires UX work, therefore it is not an on call ticket.

@JWuyts-WMF and @creynolds will come up with the requirements and Product will prioritize
cc. @Cpetrillo

I think UX, straightforward, should be:

acceptance criteria:

  • As a user, when I load the dashboard, the dashboard always shows a correct reflection of the # of calls I have left this month.
  • As a user, when I am redirected to the login page, I know why I was redirected and need to log back in.

Question if 'polling' is used on session, otherwise a stale window can stay "logged in" and without a refresh user wouldn't know. Hitting Cognito's /oauth2/userInfo endpoint at an interval like 60sec and if response.status === 401 redirect to /. At that point we can display a "Your session has expired. Please log in" 'modal' box (not a modal but the alert box I forget the name of).

RE: the refreshing of user telemetry/stats... Is that not a different ticket as it's unrelated. But that would be a longer interval poll with some caching involved there; to discuss in the/a ticket for that issue.

@creynolds @JWuyts-WMF let me nknow when the refinement of this ticket is done so I can send it to Product for the triage

Related???

Logout Race Condition - #4 in list of Security review in repo /dashboard/-/issues/15
File: src/store/useAuth.ts:52-58

const logout = async () => {
  auth.authorized = initialValue.authorized;  // Cleared before API call
  router.push("/");
  const res = await axios.post("/token-revoke", { refresh_token: auth.refresh_token });

The state is cleared and router navigates away before the token revocation API call completes. If revocation fails, the token remains valid server-side.

Fix: Move state clearing to after successful API response.

I don't believe that's related @creynolds. logout() is probably used when a user clicks the logout/signout button, but this issue occurs also the user does nothing and just lets their credentials expire.

creynolds changed the task status from Open to In Progress.Wed, Mar 18, 4:59 AM

FIxes proposed in MR46, along with that logout race condition as I was touching that part of code anyway. Need @KMontalva-WMF oversight/review/test.