ReviewOS

stacks/analyticshq

[auth] Sessions hard-expire at 1 hour — the refresh token is issued and then dropped

#32
Closed glennmichael123 opened this 9 days ago ·
9 days ago

config/auth.ts:57 sets a one-hour access token:

tokenExpiry: env.AUTH_TOKEN_EXPIRY || 60 * 60 * 1000,

config/auth.ts:60 documents a 30-day refresh token right below it. LoginAction returns one. And it is never used:

$ grep -rn 'refresh_token\|refreshToken' resources/ app/ routes/ types/ | wc -l
0

Not "used incorrectly" — zero references anywhere in the app. login.stx reads the access token off the response and discards the rest.

What the user sees

After 60 minutes the token is dead, but localStorage still holds it, so the pre-paint guard on /dashboard and /account sees a truthy value and does not bounce to /login. The server, meanwhile, cannot authenticate the request and renders the signed-out gate. The user sits on a "Sign in to view analytics" wall on a page they are signed in to, and the only way out is to sign in again.

This happens to every signed-in user, once an hour, forever.

Fix

  1. Capture refresh_token in session.signIn() alongside the access token.
  2. On a 401 from any authenticated fetch, exchange it at the existing POST /auth/refresh and retry once.
  3. Only sign out when the refresh itself fails.
  • The cookie mirror has a matching gap — see the server-set-cookie issue filed alongside this one. Both live in the same code, so do them together.
  • Worth deciding HttpOnly at the same time: @stacksjs/auth's cookie helper hardcodes HttpOnly, which document.cookie cannot overwrite, so going HttpOnly means deleting the session store's mirror effect and both pre-paint blocks rather than adding to them.
9 days ago

Fixed and pushed.

session.authFetch() now attaches the bearer header, exchanges the refresh token on a 401, and retries once. The ten call sites that hand-rolled fetch(..., { headers: authHeaders() }) go through it, and routes/auth.ts re-registers POST /auth/refresh with .skipCsrf() — the framework's copy is CSRF-gated, which blocked the same-origin fetch, exactly like the login/register/logout routes above it.

Three findings that changed the design:

The refresh token is single-use with rotation. Verified — replaying a spent one returns 401. The dashboard fires several authenticated requests at once, so one expired access token produces several simultaneous 401s. Without serialising them the first exchange wins and the rest are refused, signing the user out in the middle of a session that had just successfully renewed. refreshAccessToken() is single-flight.

A failed refresh is not always a refused one. The outcome is three-way: refused ends the session, unavailable (offline, or no refresh token at all — every share-link viewer) hands the 401 back untouched. Collapsing them would sign people out on a network blip.

RegisterAction does not return a refresh token. Measured against both endpoints rather than assumed:

response
LoginActionaccess_token, refresh_token, token_type, expires_in, token, user
RegisterAction{ token, user }

So this fix would have covered every session except that of someone who had just signed up — they would hit the wall once, on their first day. register.stx now exchanges the credentials it already holds for a full pair via /login, falling back to the register token if that call fails. Filing the asymmetry upstream separately.

Verified, not assumed

Against the running API: login returns an 80-char refresh token; /auth/refresh returns 200 (not 403/404, so the CSRF skip works); the response carries a new access token and a new refresh token; the renewed token authenticates against /api/me; the spent one is refused.

Then through the real register form in a headless browser: lands on /dashboard, fires /register then /login, and localStorage holds a correctly JSON-encoded refresh token plus a user that parses as an object. No console errors. Test accounts deleted afterwards.

Bonus, found while touching the social handoff

SocialCallbackAction was emitting localStorage.setItem('user', {"id":1,...}) — an object literal, which setItem coerces to the string "[object Object]". Every social sign-in has been storing a broken user. It also wrote the token unencoded, which only worked because the store carries a migration shim for that case. Both now write the JSON-encoded form useLocalStorage expects, with < escaped so a display name containing a closing script tag cannot break out of the inline block.

Sign in to comment on this issue.