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>
171 lines
6.4 KiB
TypeScript
171 lines
6.4 KiB
TypeScript
import type { Session } from "@remix-run/node";
|
|
import type { PrismaClientOrTransaction } from "@trigger.dev/database";
|
|
import { prisma } from "~/db.server";
|
|
import { commitSession, DEFAULT_SESSION_DURATION_SECONDS } from "./sessionStorage.server";
|
|
|
|
export { DEFAULT_SESSION_DURATION_SECONDS };
|
|
|
|
// Months and years use standard Gregorian-calendar conversions (365.2425 days/yr,
|
|
// 30.436875 days/month) so values produced by external "X months in seconds"
|
|
// calculators map cleanly to a labeled option.
|
|
const GREGORIAN_HALF_YEAR_SECONDS = 15_778_476;
|
|
|
|
export type SessionDurationOption = {
|
|
value: number;
|
|
label: string;
|
|
};
|
|
|
|
export const SESSION_DURATION_OPTIONS: SessionDurationOption[] = [
|
|
{ value: 60 * 5, label: "5 minutes" },
|
|
{ value: 60 * 30, label: "30 minutes" },
|
|
{ value: 60 * 60, label: "1 hour" },
|
|
{ value: 60 * 60 * 24, label: "1 day" },
|
|
{ value: 60 * 60 * 24 * 30, label: "30 days" },
|
|
{ value: GREGORIAN_HALF_YEAR_SECONDS, label: "6 months" },
|
|
{ value: DEFAULT_SESSION_DURATION_SECONDS, label: "1 year" },
|
|
];
|
|
|
|
export const ALLOWED_SESSION_DURATION_VALUES: ReadonlySet<number> = new Set(
|
|
SESSION_DURATION_OPTIONS.map((o) => o.value)
|
|
);
|
|
|
|
export function isAllowedSessionDuration(value: number): boolean {
|
|
return ALLOWED_SESSION_DURATION_VALUES.has(value);
|
|
}
|
|
|
|
export type OrganizationSessionCap = {
|
|
/** The org cap in seconds. */
|
|
orgCapSeconds: number;
|
|
/** The id of the org whose cap is currently the most restrictive. */
|
|
cappingOrgId: string;
|
|
};
|
|
|
|
/**
|
|
* Returns the most restrictive max session duration across the user's orgs
|
|
* along with the id of the org that owns it, ignoring orgs where the cap is
|
|
* null. Returns null when no org has set a cap.
|
|
*/
|
|
export async function getOrganizationSessionCap(
|
|
userId: string,
|
|
client: PrismaClientOrTransaction = prisma
|
|
): Promise<OrganizationSessionCap | null> {
|
|
const tightest = await client.organization.findFirst({
|
|
where: {
|
|
members: { some: { userId } },
|
|
maxSessionDuration: { not: null },
|
|
deletedAt: null,
|
|
},
|
|
orderBy: { maxSessionDuration: "asc" },
|
|
select: { id: true, maxSessionDuration: true },
|
|
});
|
|
if (!tightest || tightest.maxSessionDuration === null) return null;
|
|
return { orgCapSeconds: tightest.maxSessionDuration, cappingOrgId: tightest.id };
|
|
}
|
|
|
|
export type EffectiveSessionDuration = {
|
|
/** Effective session duration in seconds = min(user.sessionDuration, orgCap?). */
|
|
durationSeconds: number;
|
|
/** The org cap in seconds, or null if no org caps the user. */
|
|
orgCapSeconds: number | null;
|
|
/** The id of the org whose cap is currently in effect, or null. */
|
|
cappingOrgId: string | null;
|
|
/** The raw user setting in seconds. */
|
|
userSettingSeconds: number;
|
|
};
|
|
|
|
/**
|
|
* Computes the effective session duration for a user by combining their
|
|
* configured `User.sessionDuration` with the most restrictive cap across
|
|
* their organizations.
|
|
*/
|
|
export async function getEffectiveSessionDuration(
|
|
userId: string,
|
|
client: PrismaClientOrTransaction = prisma
|
|
): Promise<EffectiveSessionDuration> {
|
|
const [user, orgCap] = await Promise.all([
|
|
client.user.findFirst({
|
|
where: { id: userId },
|
|
select: { sessionDuration: true },
|
|
}),
|
|
getOrganizationSessionCap(userId, client),
|
|
]);
|
|
|
|
const userSettingSeconds = user?.sessionDuration ?? DEFAULT_SESSION_DURATION_SECONDS;
|
|
const durationSeconds =
|
|
orgCap === null ? userSettingSeconds : Math.min(userSettingSeconds, orgCap.orgCapSeconds);
|
|
|
|
return {
|
|
durationSeconds,
|
|
orgCapSeconds: orgCap?.orgCapSeconds ?? null,
|
|
cappingOrgId: orgCap?.cappingOrgId ?? null,
|
|
userSettingSeconds,
|
|
};
|
|
}
|
|
|
|
/**
|
|
* Returns the dropdown options the user is allowed to pick. Options strictly
|
|
* greater than the org cap are removed.
|
|
*
|
|
* `currentValueSeconds` should be the *effective* (clamped) duration — i.e.
|
|
* `EffectiveSessionDuration.durationSeconds`, which is guaranteed to be ≤
|
|
* `orgCapSeconds`. Passing the clamped value makes the dropdown's selected
|
|
* option reflect what's actually in effect rather than the user's stored
|
|
* preference, which is the right UX when a stricter org cap supersedes a
|
|
* larger user setting (the raw user preference stays in the DB and is
|
|
* restored automatically if the cap is later removed).
|
|
*
|
|
* The tag-along branch below — appending `currentValueSeconds` to the option
|
|
* list when it isn't already present — is now defensive only. It exists so
|
|
* that any caller passing an out-of-range value (e.g. tests, or future
|
|
* callers wanting to surface the raw user preference) still gets a renderable
|
|
* form, rather than a dropdown whose `defaultValue` matches no option.
|
|
*/
|
|
export function getAllowedSessionOptions(
|
|
orgCapSeconds: number | null,
|
|
currentValueSeconds: number
|
|
): SessionDurationOption[] {
|
|
const allowed = SESSION_DURATION_OPTIONS.filter((opt) => {
|
|
if (orgCapSeconds === null) return true;
|
|
return opt.value <= orgCapSeconds;
|
|
});
|
|
|
|
if (!allowed.some((o) => o.value === currentValueSeconds)) {
|
|
const currentLabel =
|
|
SESSION_DURATION_OPTIONS.find((o) => o.value === currentValueSeconds)?.label ??
|
|
`${currentValueSeconds} seconds`;
|
|
allowed.push({ value: currentValueSeconds, label: currentLabel });
|
|
allowed.sort((a, b) => a.value - b.value);
|
|
}
|
|
|
|
return allowed;
|
|
}
|
|
|
|
/**
|
|
* Commits the session for an authenticated user and stamps the user's
|
|
* effective expiry into `User.nextSessionEnd`. Use this at every
|
|
* login/MFA-completion point so the session window starts fresh, plus any
|
|
* time the user re-affirms their session duration. The single DB write here
|
|
* is the canonical "compute effective duration" step — request-time checks
|
|
* just read `nextSessionEnd` from the row that `requireUser`/`getUser`
|
|
* already fetches.
|
|
*
|
|
* The auth cookie's `Max-Age` is intentionally long
|
|
* (`DEFAULT_SESSION_DURATION_SECONDS`, 1 year) so the cookie always reaches
|
|
* the server. Actual session expiry is enforced server-side by reading
|
|
* `User.nextSessionEnd`. If we let the cookie expire client-side, the user
|
|
* is silently logged out.
|
|
*/
|
|
export async function commitAuthenticatedSession(
|
|
session: Session,
|
|
userId: string,
|
|
now: number = Date.now(),
|
|
client: PrismaClientOrTransaction = prisma
|
|
): Promise<string> {
|
|
const { durationSeconds } = await getEffectiveSessionDuration(userId, client);
|
|
await client.user.update({
|
|
where: { id: userId },
|
|
data: { nextSessionEnd: new Date(now + durationSeconds * 1000) },
|
|
});
|
|
return commitSession(session, { maxAge: DEFAULT_SESSION_DURATION_SECONDS });
|
|
}
|