45ec23cc73
<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>
147 lines
4.1 KiB
TypeScript
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})`;
|
|
}
|