Files
James Ritchie 45ec23cc73 feat(webapp): app auto session logout (#3473)
<img width="2284" height="2028" alt="CleanShot 2026-05-01 at 18 53
50@2x"
src="https://github.com/user-attachments/assets/4f58cbb1-0168-40fb-a523-017f2ba625a1"
/>


## Performance
- **Per-request DB hit**: `getUserId` runs `getEffectiveSessionDuration`
(User lookup + Org `aggregate`) on *every* authenticated request,
including each fetcher poll. Consider caching the effective duration in
the session cookie with a short TTL (e.g. 60s) and revalidating in the
background.
- **Double session commit in `root.tsx`**: `getUser` already runs the
expiry check; then `commitAuthenticatedSessionLazy` commits the cookie
again. Fine, but doubles `Set-Cookie` headers on every page load — worth
a quick perf check.

## Correctness / Edge cases
- **Lazy backfill assumes a root.tsx hit first**: users whose first
post-deploy request is a fetcher/API route (`/resources/*`) skip the
backfill until they navigate to a page. Not a security hole, but
`getUserId` could backfill itself for completeness.
- **No upper bound on `Organization.maxSessionDuration`**: admin API
accepts `1` second, which would instant-logout every member on next
request. Add a `min(60)` (or `min(300)` to match the lowest user option)
to the Zod schema.
- **No clock-skew tolerance**: `isSessionExpired` is exact-millisecond.
Multi-instance deploys with skewed clocks could log users out a few
seconds early/late. Probably fine for the 5-min minimum, but worth
noting.

## Security
- **Auto-logout audit log lacks IP/orgId**: HIPAA forensics typically
wants source IP and which org context. Currently logs only `userId` +
path. IP isn't PII for audit purposes; orgIds help correlate. Add both.
- **Cookie `Max-Age` is 1 year regardless of user's setting**:
intentional (server-side `issuedAt` is the source of truth), but
reviewers will ask. Add a one-line comment on the cookie config
explaining why.

## API surface
- **`maxSessionDuration` is admin-PAT only**: no in-app UI for org
owners to set/change their own cap. If this is "Trigger staff sets it
during HIPAA onboarding", say so in the PR description; otherwise add an
org-settings UI.
- **Auto-submit dropdown has no confirmation**: misclicking "5 minutes"
immediately shortens the user's session window with no undo. Consider a
save button or 3-sec undo toast.

## Schema / migration
- **`User.sessionDuration NOT NULL DEFAULT 31556952`**: instant on PG
11+ (metadata-only), but call out in the PR description so reviewers
don't worry about a table rewrite on the User table.
- **No DB-level constraint matching `SESSION_DURATION_OPTIONS`**: if the
option list changes, existing users keep orphaned values. The dropdown's
tag-along behaviour hides this — fine for now, but if you ever drop an
option you'll need a backfill.

## UX
- **Session expiry only fires on next request**: an idle authenticated
tab keeps showing UI past the cap (until SSE/polling catches it, ~60s).
Add a client-side timer based on the user's effective duration that
triggers a fetcher to `/account` or `/logout` at expiry.
- **No "you were signed out" message on logout**: users hitting their
cap are bounced to `/` with no explanation. Was intentionally reverted
in this PR — call that out so reviewers don't request it.

## Tests
- Unit coverage on `sessionDuration.server.ts` is solid (215 lines).
Missing: integration test for `getUserId` → expired session → redirect
to `/logout`, and one for the loader's clamping fix (the most recent
bug). Add at least the second one to lock in the regression.

---------

Co-authored-by: Matt Aitken <matt@mattaitken.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 15:02:26 +01:00

147 lines
4.1 KiB
TypeScript

import type { UIMatch } from "@remix-run/react";
import { useMatches } from "@remix-run/react";
const DEFAULT_REDIRECT = "/";
// Pathnames that are NOT user-navigable destinations: fetcher endpoints,
// OAuth/auth callbacks, JSON APIs, the magic-link redemption route, and the
// auth flow routes themselves (which would create a redirect loop). Note
// `/admin/api/` covers admin JSON endpoints while leaving `/admin`,
// `/admin/back-office/*`, `/admin/orgs`, etc. navigable.
const NON_NAVIGABLE_PREFIXES = ["/resources/", "/auth/", "/admin/api/", "/api/", "/engine/"];
const NON_NAVIGABLE_EXACT = new Set(["/magic", "/logout", "/login", "/login/magic", "/login/mfa"]);
function isNavigablePath(pathname: string): boolean {
if (NON_NAVIGABLE_EXACT.has(pathname)) return false;
return !NON_NAVIGABLE_PREFIXES.some((prefix) => pathname.startsWith(prefix));
}
/**
* This should be used any time the redirect path is user-provided
* (Like the query string on our login/signup pages). This avoids
* open-redirect vulnerabilities and prevents redirecting users to
* non-page routes (e.g. fetcher endpoints) that would render blank.
* @param {string} path The redirect destination
* @param {string} defaultRedirect The redirect to use if the to is unsafe.
*/
export function sanitizeRedirectPath(
path: string | undefined | null,
defaultRedirect: string = DEFAULT_REDIRECT
): string {
if (!path || typeof path !== "string") {
return defaultRedirect;
}
if (!path.startsWith("/") || path.startsWith("//")) {
return defaultRedirect;
}
try {
// should not parse as a full URL
new URL(path);
return defaultRedirect;
} catch {}
let parsed: URL;
try {
// ensure it's a valid relative path
parsed = new URL(path, "https://example.com");
if (parsed.hostname !== "example.com") {
return defaultRedirect;
}
} catch {
return defaultRedirect;
}
if (!isNavigablePath(parsed.pathname)) {
return defaultRedirect;
}
return path;
}
/**
* This base hook is used in other hooks to quickly search for specific data
* across all loader data using useMatches.
* @param {string} id The route id
* @returns {JSON|undefined} The router data or undefined if not found
*/
export function useMatchesData(id: string | string[], debug: boolean = false): UIMatch | undefined {
const matchingRoutes = useMatches();
if (debug) {
console.log("matchingRoutes", matchingRoutes);
}
const paths = Array.isArray(id) ? id : [id];
// Get the first matching route
const route = paths.reduce((acc, path) => {
if (acc) return acc;
return matchingRoutes.find((route) => route.id === path);
}, undefined as UIMatch | undefined);
return route;
}
export function validateEmail(email: unknown): email is string {
return typeof email === "string" && email.length > 3 && email.includes("@");
}
export function hydrateObject<T>(object: any): T {
return hydrateDates(object) as T;
}
export function hydrateDates(object: any): any {
if (object === null || object === undefined) {
return object;
}
if (object instanceof Date) {
return object;
}
if (
typeof object === "string" &&
object.match(/\d{4}-\d{2}-\d{2}/) &&
!Number.isNaN(Date.parse(object))
) {
return new Date(object);
}
if (typeof object === "object") {
if (Array.isArray(object)) {
return object.map((item) => hydrateDates(item));
} else {
const hydratedObject: any = {};
for (const key in object) {
hydratedObject[key] = hydrateDates(object[key]);
}
return hydratedObject;
}
}
return object;
}
export function titleCase(original: string): string {
return original
.split(" ")
.map((word) => word[0].toUpperCase() + word.slice(1))
.join(" ");
}
// Takes an api key (either trigger_live_xxxx or trigger_development_xxxx) and returns trigger_live_********
export const obfuscateApiKey = (apiKey: string) => {
const [prefix, slug, secretPart] = apiKey.split("_");
return `${prefix}_${slug}_${"*".repeat(secretPart.length)}`;
};
export function appEnvTitleTag(appEnv?: string): string {
if (!appEnv || appEnv === "production") {
return "";
}
return ` (${appEnv})`;
}