Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions docs/contributing/architecture/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,17 @@ Related handlers:
### Client session refresh behavior

The app shell (`packages/worker/client/app.tsx`) refreshes session state after
initial load and on client-side navigation events. If an in-flight refresh is
aborted, the client keeps the last known ready session instead of overwriting it
with `null`. This prevents transient logged-out UI during concurrent re-renders.
initial load and on client-side navigation events. Navigation-triggered
refreshes are throttled (30s) to avoid a `/session` round trip on every SPA
navigation, but the throttle never applies to refreshes that follow a mutation:
the client router (`packages/worker/client/client-router.tsx`) emits a
`mutation` event after every form POST it submits (exposed as
`listenToRouterMutations`), and the shell marks its session state stale so the
follow-up redirect navigation refreshes it regardless of the throttle —
auth-changing POSTs like `/logout` update the top nav immediately. If an
in-flight refresh is aborted, the client keeps the last known ready session
instead of overwriting it with `null`. This prevents transient logged-out UI
during concurrent re-renders.

## Password reset

Expand Down
13 changes: 12 additions & 1 deletion e2e/smoke.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,18 @@ test('smoke test covers shell, auth redirect, and login', async ({ page }) => {
).toBeVisible()
await expect(page.getByText('Failed to fetch')).not.toBeVisible()

await page.context().clearCookies()
// Log out through the top nav. The router intercepts the form POST and
// SPA-navigates to /login, so the shell must refresh its session state
// without a full document reload (regression: the throttled refresh kept
// the username and Log out button visible after logging out).
await page.getByRole('button', { name: 'Log out' }).click()
await expect(page).toHaveURL(/\/login$/)
await expect(page.getByRole('link', { name: 'Login' })).toBeVisible()
await expect(page.getByRole('button', { name: 'Log out' })).not.toBeVisible()
await expect(
page.getByRole('link', { name: primaryTestUser.username }),
).not.toBeVisible()

await page.goto('/privacy')
await expect(page.getByRole('heading', { name: 'Privacy' })).toBeVisible()
await expect(
Expand Down
17 changes: 15 additions & 2 deletions packages/worker/client/app.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { type Handle, css } from 'remix/ui'
import { clientRouteLoaders, clientRoutes } from './routes/index.tsx'
import {
listenToRouterMutations,
listenToRouterNavigation,
registerRouteLoaders,
Router,
Expand Down Expand Up @@ -42,12 +43,14 @@ export function App(handle: Handle<AppProps>) {
let sessionRefreshInFlight = false
let sessionRefreshQueued = false
let lastSessionRefreshAt = 0
let sessionMaybeStale = false
let currentPathname = readRouterPathname(handle)

// Navigation-triggered refreshes are throttled: auth rarely changes
// mid-session and every SPA navigation was previously a /session round
// trip (2 D1 queries). Explicit refreshes (login/logout/profile updates
// via setSessionRefreshHandler) always bypass the throttle.
// trip (2 D1 queries). Refreshes after mutations (router form POSTs such
// as logout) and explicit refreshes (profile updates via
// setSessionRefreshHandler) always bypass the throttle.
const sessionRefreshThrottleMs = 30_000

function queueSessionRefresh() {
Expand Down Expand Up @@ -80,11 +83,13 @@ export function App(handle: Handle<AppProps>) {

function queueThrottledSessionRefresh() {
if (
!sessionMaybeStale &&
sessionStatus === 'ready' &&
Date.now() - lastSessionRefreshAt < sessionRefreshThrottleMs
) {
return
}
sessionMaybeStale = false
queueSessionRefresh()
}

Expand All @@ -101,6 +106,14 @@ export function App(handle: Handle<AppProps>) {
queueThrottledSessionRefresh()
handle.update()
})
// Router form POSTs mutate server state, which may include auth (e.g.
// logout destroys the session cookie), so the next navigation must
// bypass the refresh throttle. The refresh cannot start here: the
// follow-up redirect navigation re-renders the shell, which aborts
// in-flight queued tasks and would silently drop the refresh.
listenToRouterMutations(handle, () => {
sessionMaybeStale = true
})
}

const navHomeLinkCss = {
Expand Down
22 changes: 22 additions & 0 deletions packages/worker/client/client-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,11 @@ async function submitFormThroughRouter(details: FormSubmitDetails) {
}

const response = await fetch(details.action.toString(), init)
// The server processed the mutation no matter which navigation follows
// (even if a newer navigation aborted this one). Tell listeners that
// server-derived state they cache — like the shell's session info —
// may now be stale.
routerEvents.dispatchEvent(new Event('mutation'))
if (signal.aborted) return

if (response.redirected) {
Expand Down Expand Up @@ -705,6 +710,23 @@ export function listenToRouterNavigation(
})
}

/**
* Fires after the router submits a non-GET form (a mutation, e.g. logout).
* Listeners that cache or throttle server-derived state must revalidate:
* the follow-up redirect is a SPA navigation, so nothing else re-fetches
* state the mutation may have changed.
*/
export function listenToRouterMutations(
handle: Pick<Handle, 'signal' | 'update'>,
listener: () => void,
) {
if (typeof document === 'undefined') return
ensureRouter()
addEventListeners(routerEvents, handle.signal, {
mutation: () => listener(),
})
}

export function getPathname(handle?: Pick<Handle, 'context'>) {
if (handle) {
try {
Expand Down
Loading