From 6813adf3b47e1f7d286955729be979ea8a512d6d Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 15:42:15 +0100 Subject: [PATCH 01/17] Implements new session logout --- apps/webapp/app/root.tsx | 19 +- .../app/routes/account.security/route.tsx | 31 ++- ...1.orgs.$organizationId.session-duration.ts | 32 +++ .../app/routes/auth.github.callback.tsx | 3 +- .../app/routes/auth.google.callback.tsx | 3 +- apps/webapp/app/routes/login.mfa/route.tsx | 3 +- apps/webapp/app/routes/magic.tsx | 3 +- .../SessionDurationSetting.tsx | 70 ++++++ .../route.tsx | 77 +++++++ apps/webapp/app/services/session.server.ts | 17 +- .../app/services/sessionDuration.server.ts | 181 +++++++++++++++ .../app/services/sessionStorage.server.ts | 18 +- apps/webapp/test/sessionDuration.test.ts | 213 ++++++++++++++++++ .../migration.sql | 5 + .../database/prisma/schema.prisma | 9 + 15 files changed, 669 insertions(+), 15 deletions(-) create mode 100644 apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.session-duration.ts create mode 100644 apps/webapp/app/routes/resources.account.session-duration/SessionDurationSetting.tsx create mode 100644 apps/webapp/app/routes/resources.account.session-duration/route.tsx create mode 100644 apps/webapp/app/services/sessionDuration.server.ts create mode 100644 apps/webapp/test/sessionDuration.test.ts create mode 100644 internal-packages/database/prisma/migrations/20260428140746_add_session_duration_columns/migration.sql diff --git a/apps/webapp/app/root.tsx b/apps/webapp/app/root.tsx index c6027b1a6d3..bf62104b64a 100644 --- a/apps/webapp/app/root.tsx +++ b/apps/webapp/app/root.tsx @@ -15,6 +15,8 @@ import { env } from "./env.server"; import { featuresForRequest } from "./features.server"; import { usePostHog } from "./hooks/usePostHog"; import { getUser } from "./services/session.server"; +import { commitAuthenticatedSessionLazy } from "./services/sessionDuration.server"; +import { getUserSession } from "./services/sessionStorage.server"; import { getTimezonePreference } from "./services/preferences/uiPreferences.server"; import { appEnvTitleTag } from "./utils"; @@ -58,9 +60,22 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { websiteId: env.KAPA_AI_WEBSITE_ID, }; + const user = await getUser(request); + + const headers = new Headers(); + headers.append("Set-Cookie", await commitSession(session)); + + // Lazy-backfill the auth session's `issuedAt` for cookies issued before this + // feature shipped, and refresh the cookie's Max-Age to track the user's + // current effective session duration. + if (user) { + const authSession = await getUserSession(request); + headers.append("Set-Cookie", await commitAuthenticatedSessionLazy(authSession, user.id)); + } + return typedjson( { - user: await getUser(request), + user, toastMessage, posthogProjectKey, features, @@ -70,7 +85,7 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { kapa, timezone, }, - { headers: { "Set-Cookie": await commitSession(session) } } + { headers } ); }; diff --git a/apps/webapp/app/routes/account.security/route.tsx b/apps/webapp/app/routes/account.security/route.tsx index 18cf8b54ab7..b0301b342f7 100644 --- a/apps/webapp/app/routes/account.security/route.tsx +++ b/apps/webapp/app/routes/account.security/route.tsx @@ -7,9 +7,16 @@ import { import { Header2 } from "~/components/primitives/Headers"; import { NavBar, PageTitle } from "~/components/primitives/PageHeader"; import { MfaSetup } from "../resources.account.mfa.setup/route"; +import { SessionDurationSetting } from "../resources.account.session-duration/SessionDurationSetting"; import { LoaderFunctionArgs } from "@remix-run/server-runtime"; import { requireUser } from "~/services/session.server"; import { typedjson, useTypedLoaderData } from "remix-typedjson"; +import { prisma } from "~/db.server"; +import { + getAllowedSessionOptions, + getOrganizationSessionCap, + DEFAULT_SESSION_DURATION_SECONDS, +} from "~/services/sessionDuration.server"; export const meta: MetaFunction = () => { return [ @@ -22,13 +29,28 @@ export const meta: MetaFunction = () => { export async function loader({ request }: LoaderFunctionArgs) { const user = await requireUser(request); + const [userRecord, orgCapSeconds] = await Promise.all([ + prisma.user.findUnique({ + where: { id: user.id }, + select: { sessionDuration: true }, + }), + getOrganizationSessionCap(user.id), + ]); + + const sessionDuration = userRecord?.sessionDuration ?? DEFAULT_SESSION_DURATION_SECONDS; + const sessionDurationOptions = getAllowedSessionOptions(orgCapSeconds, sessionDuration); + return typedjson({ user, + sessionDuration, + sessionDurationOptions, + orgCapSeconds, }); } export default function Page() { - const { user } = useTypedLoaderData(); + const { user, sessionDuration, sessionDurationOptions, orgCapSeconds } = + useTypedLoaderData(); return ( @@ -42,6 +64,13 @@ export default function Page() { Security +
+ +
diff --git a/apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.session-duration.ts b/apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.session-duration.ts new file mode 100644 index 00000000000..55090a1c1f1 --- /dev/null +++ b/apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.session-duration.ts @@ -0,0 +1,32 @@ +import { type ActionFunctionArgs, json } from "@remix-run/server-runtime"; +import { z } from "zod"; +import { prisma } from "~/db.server"; +import { requireAdminApiRequest } from "~/services/personalAccessToken.server"; + +const ParamsSchema = z.object({ + organizationId: z.string(), +}); + +const RequestBodySchema = z.object({ + /** + * Maximum session lifetime (seconds) for members of this organization, or + * null to remove the cap. When set, this caps each member's + * `User.sessionDuration` and is enforced on the user's next request. + */ + maxSessionDuration: z.number().int().positive().nullable(), +}); + +export async function action({ request, params }: ActionFunctionArgs) { + await requireAdminApiRequest(request); + + const { organizationId } = ParamsSchema.parse(params); + const body = RequestBodySchema.parse(await request.json()); + + const organization = await prisma.organization.update({ + where: { id: organizationId }, + data: { maxSessionDuration: body.maxSessionDuration }, + select: { id: true, slug: true, maxSessionDuration: true }, + }); + + return json({ success: true, organization }); +} diff --git a/apps/webapp/app/routes/auth.github.callback.tsx b/apps/webapp/app/routes/auth.github.callback.tsx index 2313b348f4a..4feaeb0d843 100644 --- a/apps/webapp/app/routes/auth.github.callback.tsx +++ b/apps/webapp/app/routes/auth.github.callback.tsx @@ -5,6 +5,7 @@ import { getSession, redirectWithErrorMessage } from "~/models/message.server"; import { authenticator } from "~/services/auth.server"; import { setLastAuthMethodHeader } from "~/services/lastAuthMethod.server"; import { commitSession } from "~/services/sessionStorage.server"; +import { commitAuthenticatedSession } from "~/services/sessionDuration.server"; import { trackAndClearReferralSource } from "~/services/referralSource.server"; import { redirectCookie } from "./auth.github"; import { sanitizeRedirectPath } from "~/utils"; @@ -52,7 +53,7 @@ export let loader: LoaderFunction = async ({ request }) => { session.set(authenticator.sessionKey, auth); const headers = new Headers(); - headers.append("Set-Cookie", await commitSession(session)); + headers.append("Set-Cookie", await commitAuthenticatedSession(session, auth.userId)); headers.append("Set-Cookie", await setLastAuthMethodHeader("github")); await trackAndClearReferralSource(request, auth.userId, headers); diff --git a/apps/webapp/app/routes/auth.google.callback.tsx b/apps/webapp/app/routes/auth.google.callback.tsx index 65dabd605ce..735cdab9f0f 100644 --- a/apps/webapp/app/routes/auth.google.callback.tsx +++ b/apps/webapp/app/routes/auth.google.callback.tsx @@ -5,6 +5,7 @@ import { getSession, redirectWithErrorMessage } from "~/models/message.server"; import { authenticator } from "~/services/auth.server"; import { setLastAuthMethodHeader } from "~/services/lastAuthMethod.server"; import { commitSession } from "~/services/sessionStorage.server"; +import { commitAuthenticatedSession } from "~/services/sessionDuration.server"; import { trackAndClearReferralSource } from "~/services/referralSource.server"; import { redirectCookie } from "./auth.google"; import { sanitizeRedirectPath } from "~/utils"; @@ -52,7 +53,7 @@ export let loader: LoaderFunction = async ({ request }) => { session.set(authenticator.sessionKey, auth); const headers = new Headers(); - headers.append("Set-Cookie", await commitSession(session)); + headers.append("Set-Cookie", await commitAuthenticatedSession(session, auth.userId)); headers.append("Set-Cookie", await setLastAuthMethodHeader("google")); await trackAndClearReferralSource(request, auth.userId, headers); diff --git a/apps/webapp/app/routes/login.mfa/route.tsx b/apps/webapp/app/routes/login.mfa/route.tsx index 1a71cd7227a..67006c37482 100644 --- a/apps/webapp/app/routes/login.mfa/route.tsx +++ b/apps/webapp/app/routes/login.mfa/route.tsx @@ -21,6 +21,7 @@ import { Paragraph } from "~/components/primitives/Paragraph"; import { Spinner } from "~/components/primitives/Spinner"; import { authenticator } from "~/services/auth.server"; import { commitSession, getUserSession } from "~/services/sessionStorage.server"; +import { commitAuthenticatedSession } from "~/services/sessionDuration.server"; import { getSession as getMessageSession } from "~/models/message.server"; import { MultiFactorAuthenticationService } from "~/services/mfa/multiFactorAuthentication.server"; import { redirectWithErrorMessage, redirectBackWithErrorMessage } from "~/models/message.server"; @@ -162,7 +163,7 @@ async function completeLogin(request: Request, session: Session, userId: string) session.unset("pending-mfa-redirect-to"); const headers = new Headers(); - headers.append("Set-Cookie", await commitSession(session)); + headers.append("Set-Cookie", await commitAuthenticatedSession(session, userId)); await trackAndClearReferralSource(request, userId, headers); diff --git a/apps/webapp/app/routes/magic.tsx b/apps/webapp/app/routes/magic.tsx index 682f0ef46e5..b205c1e4230 100644 --- a/apps/webapp/app/routes/magic.tsx +++ b/apps/webapp/app/routes/magic.tsx @@ -6,6 +6,7 @@ import { authenticator } from "~/services/auth.server"; import { setLastAuthMethodHeader } from "~/services/lastAuthMethod.server"; import { getRedirectTo } from "~/services/redirectTo.server"; import { commitSession, getSession } from "~/services/sessionStorage.server"; +import { commitAuthenticatedSession } from "~/services/sessionDuration.server"; import { trackAndClearReferralSource } from "~/services/referralSource.server"; export async function loader({ request }: LoaderFunctionArgs) { @@ -51,7 +52,7 @@ export async function loader({ request }: LoaderFunctionArgs) { session.set(authenticator.sessionKey, auth); const headers = new Headers(); - headers.append("Set-Cookie", await commitSession(session)); + headers.append("Set-Cookie", await commitAuthenticatedSession(session, auth.userId)); headers.append("Set-Cookie", await setLastAuthMethodHeader("email")); await trackAndClearReferralSource(request, auth.userId, headers); diff --git a/apps/webapp/app/routes/resources.account.session-duration/SessionDurationSetting.tsx b/apps/webapp/app/routes/resources.account.session-duration/SessionDurationSetting.tsx new file mode 100644 index 00000000000..05ea1c0ba40 --- /dev/null +++ b/apps/webapp/app/routes/resources.account.session-duration/SessionDurationSetting.tsx @@ -0,0 +1,70 @@ +import { Form, useNavigation } from "@remix-run/react"; +import { Button } from "~/components/primitives/Buttons"; +import { InputGroup } from "~/components/primitives/InputGroup"; +import { Label } from "~/components/primitives/Label"; +import { Paragraph } from "~/components/primitives/Paragraph"; +import { Select, SelectItem } from "~/components/primitives/Select"; +import type { SessionDurationOption } from "~/services/sessionDuration.server"; + +interface SessionDurationSettingProps { + currentValue: number; + options: SessionDurationOption[]; + orgCapSeconds: number | null; +} + +export function SessionDurationSetting({ + currentValue, + options, + orgCapSeconds, +}: SessionDurationSettingProps) { + const navigation = useNavigation(); + const isSubmitting = + navigation.state !== "idle" && + navigation.formAction === "/resources/account/session-duration"; + + const orgCapOption = + orgCapSeconds === null ? null : options.find((o) => o.value === orgCapSeconds); + + return ( +
+ + + + Automatically log out after this period of time. Choose a shorter duration for added + security on shared or unattended devices. + {orgCapSeconds !== null ? ( + <> + {" "} + Your organization caps this at {orgCapOption?.label ?? `${orgCapSeconds} seconds`}. + + ) : null} + + +
+ + +
+
+ ); +} diff --git a/apps/webapp/app/routes/resources.account.session-duration/route.tsx b/apps/webapp/app/routes/resources.account.session-duration/route.tsx new file mode 100644 index 00000000000..edf617e73fa --- /dev/null +++ b/apps/webapp/app/routes/resources.account.session-duration/route.tsx @@ -0,0 +1,77 @@ +import { redirect, type ActionFunctionArgs } from "@remix-run/server-runtime"; +import { z } from "zod"; +import { prisma } from "~/db.server"; +import { + commitSession as commitMessageSession, + getSession as getMessageSession, + setErrorMessage, + setSuccessMessage, +} from "~/models/message.server"; +import { requireUserId } from "~/services/session.server"; +import { + commitAuthenticatedSession, + getAllowedSessionOptions, + getEffectiveSessionDuration, + isAllowedSessionDuration, +} from "~/services/sessionDuration.server"; +import { getUserSession } from "~/services/sessionStorage.server"; + +const FormSchema = z.object({ + sessionDuration: z.coerce.number().int().positive(), +}); + +const REDIRECT_PATH = "/account/security"; + +async function redirectWithError(request: Request, message: string) { + const messageSession = await getMessageSession(request.headers.get("cookie")); + setErrorMessage(messageSession, message); + return redirect(REDIRECT_PATH, { + headers: { "Set-Cookie": await commitMessageSession(messageSession) }, + }); +} + +export async function action({ request }: ActionFunctionArgs) { + const userId = await requireUserId(request); + + const formData = await request.formData(); + const parsed = FormSchema.safeParse(Object.fromEntries(formData)); + + if (!parsed.success) { + return redirectWithError(request, "Invalid session duration value."); + } + + const { sessionDuration } = parsed.data; + + if (!isAllowedSessionDuration(sessionDuration)) { + return redirectWithError(request, "Invalid session duration value."); + } + + const { orgCapSeconds, userSettingSeconds } = await getEffectiveSessionDuration(userId); + const allowed = getAllowedSessionOptions(orgCapSeconds, userSettingSeconds); + if (!allowed.some((o) => o.value === sessionDuration)) { + return redirectWithError( + request, + "Your organization's policy does not allow that session duration." + ); + } + + await prisma.user.update({ + where: { id: userId }, + data: { sessionDuration }, + }); + + // Re-issue the cookie with the new maxAge and reset issuedAt so the user + // gets a fresh window matching their new selection right away. + const authSession = await getUserSession(request); + const authCookie = await commitAuthenticatedSession(authSession, userId); + + const messageSession = await getMessageSession(request.headers.get("cookie")); + setSuccessMessage(messageSession, "Session duration updated."); + const messageCookie = await commitMessageSession(messageSession); + + const headers = new Headers(); + headers.append("Set-Cookie", authCookie); + headers.append("Set-Cookie", messageCookie); + + return redirect(REDIRECT_PATH, { headers }); +} diff --git a/apps/webapp/app/services/session.server.ts b/apps/webapp/app/services/session.server.ts index ea6831265c7..e792e4caa51 100644 --- a/apps/webapp/app/services/session.server.ts +++ b/apps/webapp/app/services/session.server.ts @@ -2,6 +2,8 @@ import { redirect } from "@remix-run/node"; import { getUserById } from "~/models/user.server"; import { authenticator } from "./auth.server"; import { getImpersonationId } from "./impersonation.server"; +import { getEffectiveSessionDuration, isSessionExpired } from "./sessionDuration.server"; +import { getUserSession } from "./sessionStorage.server"; export async function getUserId(request: Request): Promise { const impersonatedUserId = await getImpersonationId(request); @@ -19,8 +21,19 @@ export async function getUserId(request: Request): Promise { return authUser?.userId; } - let authUser = await authenticator.isAuthenticated(request); - return authUser?.userId; + const authUser = await authenticator.isAuthenticated(request); + if (!authUser?.userId) return undefined; + + // Enforce the user's effective session duration (User.sessionDuration capped + // by the most restrictive Organization.maxSessionDuration). If the session + // was issued longer ago than the cap allows, force a logout. + const session = await getUserSession(request); + const { durationSeconds } = await getEffectiveSessionDuration(authUser.userId); + if (isSessionExpired(session, durationSeconds)) { + throw await logout(request); + } + + return authUser.userId; } export async function getUser(request: Request) { diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts new file mode 100644 index 00000000000..83c7fadfd7b --- /dev/null +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -0,0 +1,181 @@ +import type { Session } from "@remix-run/node"; +import type { PrismaClientOrTransaction } from "@trigger.dev/database"; +import { prisma } from "~/db.server"; +import { commitSession } from "./sessionStorage.server"; + +export const SESSION_ISSUED_AT_KEY = "session:issuedAt"; + +export const DEFAULT_SESSION_DURATION_SECONDS = 60 * 60 * 24 * 365; + +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: 60 * 60 * 24 * 30 * 6, label: "6 months" }, + { value: 60 * 60 * 24 * 365, label: "1 year" }, +]; + +export const ALLOWED_SESSION_DURATION_VALUES: ReadonlySet = new Set( + SESSION_DURATION_OPTIONS.map((o) => o.value) +); + +export function isAllowedSessionDuration(value: number): boolean { + return ALLOWED_SESSION_DURATION_VALUES.has(value); +} + +/** + * Returns the most restrictive max session duration (in seconds) across all of + * the user's organizations, ignoring orgs where it's null. Returns null when + * no org has set a cap. + */ +export async function getOrganizationSessionCap( + userId: string, + client: PrismaClientOrTransaction = prisma +): Promise { + const result = await client.organization.aggregate({ + where: { + members: { some: { userId } }, + maxSessionDuration: { not: null }, + deletedAt: null, + }, + _min: { maxSessionDuration: true }, + }); + return result._min.maxSessionDuration ?? null; +} + +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 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 { + const [user, orgCap] = await Promise.all([ + client.user.findUnique({ + 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); + + return { + durationSeconds, + orgCapSeconds: orgCap, + userSettingSeconds, + }; +} + +/** + * Returns the dropdown options the user is allowed to pick. If an org cap + * exists, options strictly greater than the cap are removed. The user's + * currently-saved value is always included even if it now exceeds the cap, so + * the form remains valid until they pick a smaller value. + */ +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; +} + +function readSessionIssuedAt(session: Session): number | null { + const raw = session.get(SESSION_ISSUED_AT_KEY); + if (typeof raw !== "number" || !Number.isFinite(raw)) return null; + return raw; +} + +/** + * Returns true when the session has an issuedAt timestamp older than the + * effective duration. Missing issuedAt is treated as not expired (legacy + * cookies from before this feature shipped will be lazily backfilled). + */ +export function isSessionExpired( + session: Session, + effectiveDurationSeconds: number, + now: number = Date.now() +): boolean { + const issuedAt = readSessionIssuedAt(session); + if (issuedAt === null) return false; + return now - issuedAt > effectiveDurationSeconds * 1000; +} + +/** Sets the session's issuedAt to `now` (epoch ms). */ +export function setSessionIssuedAt(session: Session, now: number = Date.now()): void { + session.set(SESSION_ISSUED_AT_KEY, now); +} + +/** + * If the session has no issuedAt set, sets it to `now` and returns true so the + * caller knows to commit the cookie. Returns false when nothing changed. + */ +export function ensureSessionIssuedAt(session: Session, now: number = Date.now()): boolean { + if (readSessionIssuedAt(session) !== null) return false; + setSessionIssuedAt(session, now); + return true; +} + +/** + * Commits the session for an authenticated user, setting `issuedAt = now` and + * the cookie's `Max-Age` to the effective session duration. Use this at every + * login/MFA-completion point so the session window starts fresh. + */ +export async function commitAuthenticatedSession( + session: Session, + userId: string, + now: number = Date.now() +): Promise { + const { durationSeconds } = await getEffectiveSessionDuration(userId); + setSessionIssuedAt(session, now); + return commitSession(session, { maxAge: durationSeconds }); +} + +/** + * Commits the session for an authenticated user, lazily backfilling + * `issuedAt` if missing. Use on every authenticated response that already + * commits the cookie (e.g. root.tsx) so legacy cookies migrate forward and + * the browser's stored Max-Age tracks the latest effective duration. + */ +export async function commitAuthenticatedSessionLazy( + session: Session, + userId: string, + now: number = Date.now() +): Promise { + const { durationSeconds } = await getEffectiveSessionDuration(userId); + ensureSessionIssuedAt(session, now); + return commitSession(session, { maxAge: durationSeconds }); +} diff --git a/apps/webapp/app/services/sessionStorage.server.ts b/apps/webapp/app/services/sessionStorage.server.ts index 3bb9a51fa7e..90e7d069abb 100644 --- a/apps/webapp/app/services/sessionStorage.server.ts +++ b/apps/webapp/app/services/sessionStorage.server.ts @@ -1,15 +1,21 @@ import { createCookieSessionStorage } from "@remix-run/node"; import { env } from "~/env.server"; +// Hard ceiling for the cookie lifetime. The actual per-session value is set +// per-commit via commitSession(session, { maxAge }) in the auth/login flows +// and on every authenticated response, derived from the user's effective +// session duration (User.sessionDuration capped by Organization.maxSessionDuration). +export const SESSION_STORAGE_MAX_AGE_SECONDS = 60 * 60 * 24 * 365; + export const sessionStorage = createCookieSessionStorage({ cookie: { - name: "__session", // use any name you want here - sameSite: "lax", // this helps with CSRF - path: "/", // remember to add this so the cookie will work in all routes - httpOnly: true, // for security reasons, make this cookie http only + name: "__session", + sameSite: "lax", + path: "/", + httpOnly: true, secrets: [env.SESSION_SECRET], - secure: env.NODE_ENV === "production", // enable this in prod only - maxAge: 60 * 60 * 24 * 365, // 7 days + secure: env.NODE_ENV === "production", + maxAge: SESSION_STORAGE_MAX_AGE_SECONDS, }, }); diff --git a/apps/webapp/test/sessionDuration.test.ts b/apps/webapp/test/sessionDuration.test.ts new file mode 100644 index 00000000000..1592fe0a229 --- /dev/null +++ b/apps/webapp/test/sessionDuration.test.ts @@ -0,0 +1,213 @@ +import { containerTest } from "@internal/testcontainers"; +import { createCookieSessionStorage, type Session } from "@remix-run/node"; +import { describe, expect, it } from "vitest"; +import { + DEFAULT_SESSION_DURATION_SECONDS, + ensureSessionIssuedAt, + getAllowedSessionOptions, + getEffectiveSessionDuration, + getOrganizationSessionCap, + isAllowedSessionDuration, + isSessionExpired, + SESSION_DURATION_OPTIONS, + SESSION_ISSUED_AT_KEY, + setSessionIssuedAt, +} from "../app/services/sessionDuration.server"; + +const oneHour = 60 * 60; +const oneDay = 60 * 60 * 24; +const oneYear = 60 * 60 * 24 * 365; + +const sessionStorage = createCookieSessionStorage({ + cookie: { name: "__test_session", secrets: ["test"] }, +}); + +async function makeEmptySession(): Promise { + return sessionStorage.getSession(); +} + +describe("isAllowedSessionDuration", () => { + it("accepts every value in the dropdown options", () => { + for (const option of SESSION_DURATION_OPTIONS) { + expect(isAllowedSessionDuration(option.value)).toBe(true); + } + }); + + it("rejects values not in the dropdown", () => { + expect(isAllowedSessionDuration(1)).toBe(false); + expect(isAllowedSessionDuration(7 * oneDay)).toBe(false); + expect(isAllowedSessionDuration(0)).toBe(false); + expect(isAllowedSessionDuration(-1)).toBe(false); + }); +}); + +describe("getAllowedSessionOptions", () => { + it("returns all options when there is no org cap", () => { + const options = getAllowedSessionOptions(null, oneYear); + expect(options).toEqual(SESSION_DURATION_OPTIONS); + }); + + it("filters out options larger than the org cap", () => { + const options = getAllowedSessionOptions(oneHour, oneHour); + expect(options.map((o) => o.value)).toEqual([60 * 5, 60 * 30, 60 * 60]); + }); + + it("includes the user's current value even when it exceeds the cap, so the form stays valid", () => { + const options = getAllowedSessionOptions(oneHour, oneYear); + expect(options.some((o) => o.value === oneYear)).toBe(true); + expect(options.some((o) => o.value === oneHour)).toBe(true); + }); + + it("does not duplicate the current value when it is already within the cap", () => { + const options = getAllowedSessionOptions(oneDay, oneHour); + const oneHourCount = options.filter((o) => o.value === oneHour).length; + expect(oneHourCount).toBe(1); + }); +}); + +describe("isSessionExpired", () => { + it("returns false when issuedAt is missing (legacy cookie)", async () => { + const session = await makeEmptySession(); + expect(isSessionExpired(session, oneHour)).toBe(false); + }); + + it("returns false when within the duration window", async () => { + const session = await makeEmptySession(); + const now = 1_000_000_000_000; + setSessionIssuedAt(session, now - 60 * 1000); + expect(isSessionExpired(session, oneHour, now)).toBe(false); + }); + + it("returns true when older than the duration window", async () => { + const session = await makeEmptySession(); + const now = 1_000_000_000_000; + setSessionIssuedAt(session, now - (oneHour + 1) * 1000); + expect(isSessionExpired(session, oneHour, now)).toBe(true); + }); +}); + +describe("ensureSessionIssuedAt", () => { + it("sets issuedAt and returns true when missing", async () => { + const session = await makeEmptySession(); + const now = 1_700_000_000_000; + expect(ensureSessionIssuedAt(session, now)).toBe(true); + expect(session.get(SESSION_ISSUED_AT_KEY)).toBe(now); + }); + + it("leaves issuedAt unchanged and returns false when already set", async () => { + const session = await makeEmptySession(); + const original = 1_500_000_000_000; + setSessionIssuedAt(session, original); + expect(ensureSessionIssuedAt(session, 1_700_000_000_000)).toBe(false); + expect(session.get(SESSION_ISSUED_AT_KEY)).toBe(original); + }); +}); + +async function createUser(prisma: any, email: string, sessionDuration?: number) { + return prisma.user.create({ + data: { + email, + authenticationMethod: "MAGIC_LINK", + ...(sessionDuration !== undefined ? { sessionDuration } : {}), + }, + }); +} + +async function createOrgWithMember( + prisma: any, + slug: string, + userId: string, + maxSessionDuration: number | null +) { + return prisma.organization.create({ + data: { + title: `Org ${slug}`, + slug, + maxSessionDuration, + members: { create: { userId, role: "ADMIN" } }, + }, + }); +} + +describe("getOrganizationSessionCap", () => { + containerTest("returns null when the user has no orgs with a cap set", async ({ prisma }) => { + const user = await createUser(prisma, "no-cap@test.com"); + await createOrgWithMember(prisma, "no-cap-org", user.id, null); + + const cap = await getOrganizationSessionCap(user.id, prisma); + expect(cap).toBeNull(); + }); + + containerTest( + "returns the most restrictive cap across orgs, ignoring nulls", + async ({ prisma }) => { + const user = await createUser(prisma, "multi-org@test.com"); + await createOrgWithMember(prisma, "loose-org", user.id, oneDay); + await createOrgWithMember(prisma, "tight-org", user.id, oneHour); + await createOrgWithMember(prisma, "uncapped-org", user.id, null); + + const cap = await getOrganizationSessionCap(user.id, prisma); + expect(cap).toBe(oneHour); + } + ); + + containerTest("ignores soft-deleted organizations", async ({ prisma }) => { + const user = await createUser(prisma, "deleted-org-user@test.com"); + const tight = await createOrgWithMember(prisma, "deleted-tight", user.id, oneHour); + await createOrgWithMember(prisma, "active-loose", user.id, oneDay); + + await prisma.organization.update({ + where: { id: tight.id }, + data: { deletedAt: new Date() }, + }); + + const cap = await getOrganizationSessionCap(user.id, prisma); + expect(cap).toBe(oneDay); + }); +}); + +describe("getEffectiveSessionDuration", () => { + containerTest( + "returns the user setting when no org cap is set", + async ({ prisma }) => { + const user = await createUser(prisma, "effective-no-cap@test.com", oneDay); + await createOrgWithMember(prisma, "effective-no-cap-org", user.id, null); + + const result = await getEffectiveSessionDuration(user.id, prisma); + expect(result.userSettingSeconds).toBe(oneDay); + expect(result.orgCapSeconds).toBeNull(); + expect(result.durationSeconds).toBe(oneDay); + } + ); + + containerTest("caps the user setting at the most restrictive org cap", async ({ prisma }) => { + const user = await createUser(prisma, "effective-capped@test.com", oneYear); + await createOrgWithMember(prisma, "effective-capped-org", user.id, oneHour); + + const result = await getEffectiveSessionDuration(user.id, prisma); + expect(result.userSettingSeconds).toBe(oneYear); + expect(result.orgCapSeconds).toBe(oneHour); + expect(result.durationSeconds).toBe(oneHour); + }); + + containerTest( + "returns the user setting when it is already smaller than the org cap", + async ({ prisma }) => { + const user = await createUser(prisma, "effective-user-smaller@test.com", 60 * 5); + await createOrgWithMember(prisma, "effective-user-smaller-org", user.id, oneHour); + + const result = await getEffectiveSessionDuration(user.id, prisma); + expect(result.durationSeconds).toBe(60 * 5); + } + ); + + containerTest( + "uses the default when the user has no row (defensive fallback)", + async ({ prisma }) => { + const result = await getEffectiveSessionDuration("nonexistent-user-id", prisma); + expect(result.userSettingSeconds).toBe(DEFAULT_SESSION_DURATION_SECONDS); + expect(result.orgCapSeconds).toBeNull(); + expect(result.durationSeconds).toBe(DEFAULT_SESSION_DURATION_SECONDS); + } + ); +}); diff --git a/internal-packages/database/prisma/migrations/20260428140746_add_session_duration_columns/migration.sql b/internal-packages/database/prisma/migrations/20260428140746_add_session_duration_columns/migration.sql new file mode 100644 index 00000000000..cf980e63e9e --- /dev/null +++ b/internal-packages/database/prisma/migrations/20260428140746_add_session_duration_columns/migration.sql @@ -0,0 +1,5 @@ +-- AlterTable +ALTER TABLE "public"."User" ADD COLUMN "sessionDuration" INTEGER NOT NULL DEFAULT 31536000; + +-- AlterTable +ALTER TABLE "public"."Organization" ADD COLUMN "maxSessionDuration" INTEGER; diff --git a/internal-packages/database/prisma/schema.prisma b/internal-packages/database/prisma/schema.prisma index 9ccf2495d3a..c0f25efc5bb 100644 --- a/internal-packages/database/prisma/schema.prisma +++ b/internal-packages/database/prisma/schema.prisma @@ -54,6 +54,10 @@ model User { /// Hash of the last used code to prevent replay attacks mfaLastUsedCode String? + /// Maximum session lifetime in seconds for this user. Default = 1 year. + /// May be further restricted by Organization.maxSessionDuration on any of the user's orgs. + sessionDuration Int @default(31536000) + invitationCode InvitationCode? @relation(fields: [invitationCodeId], references: [id]) invitationCodeId String? personalAccessTokens PersonalAccessToken[] @@ -220,6 +224,11 @@ model Organization { maximumProjectCount Int @default(25) + /// Maximum session lifetime in seconds for members of this organization. + /// When set, caps each member's User.sessionDuration. The most restrictive cap across + /// all of a user's orgs (excluding nulls) wins. + maxSessionDuration Int? + projects Project[] members OrgMember[] invites OrgMemberInvite[] From 2bc0b77271da1678147bc5294e1548d854f2897d Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 16:24:16 +0100 Subject: [PATCH 02/17] Submit the form without a save button --- .../SessionDurationSetting.tsx | 91 ++++++++++--------- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/apps/webapp/app/routes/resources.account.session-duration/SessionDurationSetting.tsx b/apps/webapp/app/routes/resources.account.session-duration/SessionDurationSetting.tsx index 05ea1c0ba40..299b7877df2 100644 --- a/apps/webapp/app/routes/resources.account.session-duration/SessionDurationSetting.tsx +++ b/apps/webapp/app/routes/resources.account.session-duration/SessionDurationSetting.tsx @@ -1,5 +1,4 @@ -import { Form, useNavigation } from "@remix-run/react"; -import { Button } from "~/components/primitives/Buttons"; +import { useSubmit } from "@remix-run/react"; import { InputGroup } from "~/components/primitives/InputGroup"; import { Label } from "~/components/primitives/Label"; import { Paragraph } from "~/components/primitives/Paragraph"; @@ -17,54 +16,56 @@ export function SessionDurationSetting({ options, orgCapSeconds, }: SessionDurationSettingProps) { - const navigation = useNavigation(); - const isSubmitting = - navigation.state !== "idle" && - navigation.formAction === "/resources/account/session-duration"; + const submit = useSubmit(); const orgCapOption = orgCapSeconds === null ? null : options.find((o) => o.value === orgCapSeconds); return ( -
- - - - Automatically log out after this period of time. Choose a shorter duration for added - security on shared or unattended devices. - {orgCapSeconds !== null ? ( - <> - {" "} - Your organization caps this at {orgCapOption?.label ?? `${orgCapSeconds} seconds`}. - - ) : null} - - -
- - +
+
+ + + + Automatically log out after a period of time. + {orgCapSeconds !== null ? ( + <> + {" "} + Your organization caps this at {orgCapOption?.label ?? `${orgCapSeconds} seconds`}. + + ) : null} + + +
+ +
- +
); } From 83e779dc0f4994c5b213ba5fabf8cf2c91ccad09 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 16:24:43 +0100 Subject: [PATCH 03/17] Better settings layout --- .../app/routes/account.security/route.tsx | 22 ++++++------ .../resources.account.mfa.setup/MfaToggle.tsx | 36 +++++++++---------- .../resources.account.mfa.setup/route.tsx | 18 +++++----- apps/webapp/test/sessionDuration.test.ts | 4 ++- 4 files changed, 43 insertions(+), 37 deletions(-) diff --git a/apps/webapp/app/routes/account.security/route.tsx b/apps/webapp/app/routes/account.security/route.tsx index b0301b342f7..02ce3f9d776 100644 --- a/apps/webapp/app/routes/account.security/route.tsx +++ b/apps/webapp/app/routes/account.security/route.tsx @@ -1,4 +1,6 @@ import { type MetaFunction } from "@remix-run/react"; +import { LoaderFunctionArgs } from "@remix-run/server-runtime"; +import { typedjson, useTypedLoaderData } from "remix-typedjson"; import { MainHorizontallyCenteredContainer, PageBody, @@ -6,17 +8,15 @@ import { } from "~/components/layout/AppLayout"; import { Header2 } from "~/components/primitives/Headers"; import { NavBar, PageTitle } from "~/components/primitives/PageHeader"; -import { MfaSetup } from "../resources.account.mfa.setup/route"; -import { SessionDurationSetting } from "../resources.account.session-duration/SessionDurationSetting"; -import { LoaderFunctionArgs } from "@remix-run/server-runtime"; -import { requireUser } from "~/services/session.server"; -import { typedjson, useTypedLoaderData } from "remix-typedjson"; import { prisma } from "~/db.server"; +import { requireUser } from "~/services/session.server"; import { + DEFAULT_SESSION_DURATION_SECONDS, getAllowedSessionOptions, getOrganizationSessionCap, - DEFAULT_SESSION_DURATION_SECONDS, } from "~/services/sessionDuration.server"; +import { MfaSetup } from "../resources.account.mfa.setup/route"; +import { SessionDurationSetting } from "../resources.account.session-duration/SessionDurationSetting"; export const meta: MetaFunction = () => { return [ @@ -59,12 +59,14 @@ export default function Page() { - -
+ +
Security
- -
+
+ +
+
- - - - Enable an extra layer of security by requiring a one-time code from your authenticator - app (TOTP) each time you log in. - - -
- +
+ + + + Require a one-time code from your authenticator app (TOTP). + + +
+ +
); -} \ No newline at end of file +} diff --git a/apps/webapp/app/routes/resources.account.mfa.setup/route.tsx b/apps/webapp/app/routes/resources.account.mfa.setup/route.tsx index 33800cb842f..04f5c8f74e0 100644 --- a/apps/webapp/app/routes/resources.account.mfa.setup/route.tsx +++ b/apps/webapp/app/routes/resources.account.mfa.setup/route.tsx @@ -1,7 +1,11 @@ -import { ActionFunctionArgs } from "@remix-run/server-runtime"; +import { type ActionFunctionArgs } from "@remix-run/server-runtime"; import { typedjson } from "remix-typedjson"; import { z } from "zod"; -import { redirectWithSuccessMessage, redirectWithErrorMessage, typedJsonWithSuccessMessage } from "~/models/message.server"; +import { + redirectWithSuccessMessage, + redirectWithErrorMessage, + typedJsonWithSuccessMessage, +} from "~/models/message.server"; import { MultiFactorAuthenticationService } from "~/services/mfa/multiFactorAuthentication.server"; import { requireUserId } from "~/services/session.server"; import { ServiceValidationError } from "~/v3/services/baseService.server"; @@ -132,14 +136,15 @@ export async function action({ request }: ActionFunctionArgs) { if (error instanceof ServiceValidationError) { return redirectWithErrorMessage("/account/security", request, error.message); } - + // Re-throw unexpected errors throw error; } } export function MfaSetup({ isEnabled }: { isEnabled: boolean }) { - const { state, actions, isQrDialogOpen, isRecoveryDialogOpen, isDisableDialogOpen } = useMfaSetup(isEnabled); + const { state, actions, isQrDialogOpen, isRecoveryDialogOpen, isDisableDialogOpen } = + useMfaSetup(isEnabled); const handleToggle = (enabled: boolean) => { if (enabled && !state.isEnabled) { @@ -151,10 +156,7 @@ export function MfaSetup({ isEnabled }: { isEnabled: boolean }) { return ( <> - + Date: Tue, 28 Apr 2026 16:34:56 +0100 Subject: [PATCH 04/17] Show friendly labels for capped duration values --- apps/webapp/app/services/sessionDuration.server.ts | 12 +++++++++--- apps/webapp/test/sessionDuration.test.ts | 2 +- .../migration.sql | 2 ++ internal-packages/database/prisma/schema.prisma | 4 ++-- 4 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 internal-packages/database/prisma/migrations/20260428152919_use_gregorian_year_for_user_session_duration_default/migration.sql diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index 83c7fadfd7b..7afbd60db8b 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -5,7 +5,13 @@ import { commitSession } from "./sessionStorage.server"; export const SESSION_ISSUED_AT_KEY = "session:issuedAt"; -export const DEFAULT_SESSION_DURATION_SECONDS = 60 * 60 * 24 * 365; +// 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_YEAR_SECONDS = 31_556_952; // 365.2425 * 86400 +const GREGORIAN_HALF_YEAR_SECONDS = 15_778_476; + +export const DEFAULT_SESSION_DURATION_SECONDS = GREGORIAN_YEAR_SECONDS; export type SessionDurationOption = { value: number; @@ -18,8 +24,8 @@ export const SESSION_DURATION_OPTIONS: SessionDurationOption[] = [ { value: 60 * 60, label: "1 hour" }, { value: 60 * 60 * 24, label: "1 day" }, { value: 60 * 60 * 24 * 30, label: "30 days" }, - { value: 60 * 60 * 24 * 30 * 6, label: "6 months" }, - { value: 60 * 60 * 24 * 365, label: "1 year" }, + { value: GREGORIAN_HALF_YEAR_SECONDS, label: "6 months" }, + { value: GREGORIAN_YEAR_SECONDS, label: "1 year" }, ]; export const ALLOWED_SESSION_DURATION_VALUES: ReadonlySet = new Set( diff --git a/apps/webapp/test/sessionDuration.test.ts b/apps/webapp/test/sessionDuration.test.ts index 7cb0e8a158a..17b4b932c7e 100644 --- a/apps/webapp/test/sessionDuration.test.ts +++ b/apps/webapp/test/sessionDuration.test.ts @@ -18,7 +18,7 @@ import { const oneHour = 60 * 60; const oneDay = 60 * 60 * 24; -const oneYear = 60 * 60 * 24 * 365; +const oneYear = DEFAULT_SESSION_DURATION_SECONDS; const sessionStorage = createCookieSessionStorage({ cookie: { name: "__test_session", secrets: ["test"] }, diff --git a/internal-packages/database/prisma/migrations/20260428152919_use_gregorian_year_for_user_session_duration_default/migration.sql b/internal-packages/database/prisma/migrations/20260428152919_use_gregorian_year_for_user_session_duration_default/migration.sql new file mode 100644 index 00000000000..83cdb946158 --- /dev/null +++ b/internal-packages/database/prisma/migrations/20260428152919_use_gregorian_year_for_user_session_duration_default/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "public"."User" ALTER COLUMN "sessionDuration" SET DEFAULT 31556952; diff --git a/internal-packages/database/prisma/schema.prisma b/internal-packages/database/prisma/schema.prisma index c0f25efc5bb..aecb6853372 100644 --- a/internal-packages/database/prisma/schema.prisma +++ b/internal-packages/database/prisma/schema.prisma @@ -54,9 +54,9 @@ model User { /// Hash of the last used code to prevent replay attacks mfaLastUsedCode String? - /// Maximum session lifetime in seconds for this user. Default = 1 year. + /// Maximum session lifetime in seconds for this user. Default = 1 year (Gregorian). /// May be further restricted by Organization.maxSessionDuration on any of the user's orgs. - sessionDuration Int @default(31536000) + sessionDuration Int @default(31556952) invitationCode InvitationCode? @relation(fields: [invitationCodeId], references: [id]) invitationCodeId String? From a3d72394fc534d94b9190eb72a6640707040bab9 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 17:01:08 +0100 Subject: [PATCH 05/17] Bug fix: When getting logged out, logging back in redirected you to /resources/platform-changelogs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause recap: Background fetcher (useRecentChangelogs polling /resources/platform-changelogs every 60s) was the first thing to hit requireUserId after a session loss. That stuffed its own URL into ?redirectTo=..., the login flow stored it in a cookie, and /magic faithfully redirected the user to a JSON-only fetcher route after sign-in → blank page. Pre-existing bug, surfaced more often by the new session-expiry feature. Fixed by sanitizing redirectTo at three layers. --- apps/webapp/app/routes/login.magic/route.tsx | 10 ++++++--- apps/webapp/app/routes/magic.tsx | 6 ++++- apps/webapp/app/services/session.server.ts | 9 +++++--- apps/webapp/app/utils.ts | 23 +++++++++++++++++--- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/apps/webapp/app/routes/login.magic/route.tsx b/apps/webapp/app/routes/login.magic/route.tsx index 8c4323be54e..3ddbd47a4d0 100644 --- a/apps/webapp/app/routes/login.magic/route.tsx +++ b/apps/webapp/app/routes/login.magic/route.tsx @@ -23,6 +23,7 @@ import { TextLink } from "~/components/primitives/TextLink"; import { authenticator } from "~/services/auth.server"; import { commitSession, getUserSession } from "~/services/sessionStorage.server"; import { setRedirectTo, commitSession as commitRedirectSession } from "~/services/redirectTo.server"; +import { sanitizeRedirectPath } from "~/utils"; import { checkMagicLinkEmailRateLimit, checkMagicLinkEmailDailyRateLimit, @@ -60,11 +61,14 @@ export async function loader({ request }: LoaderFunctionArgs) { const session = await getUserSession(request); const error = session.get("auth:error"); - // Get redirectTo from URL params and store in session if present + // Get redirectTo from URL params and store in session if present. + // Sanitize to drop non-page paths (fetcher routes, callbacks) which would + // render blank if the user was sent there post-login. const url = new URL(request.url); - const redirectTo = url.searchParams.get("redirectTo"); + const sanitized = sanitizeRedirectPath(url.searchParams.get("redirectTo")); + const redirectTo = sanitized === "/" ? null : sanitized; const headers = new Headers(); - + if (redirectTo) { const redirectSession = await setRedirectTo(request, redirectTo); headers.append("Set-Cookie", await commitRedirectSession(redirectSession)); diff --git a/apps/webapp/app/routes/magic.tsx b/apps/webapp/app/routes/magic.tsx index b205c1e4230..d4606e1b7de 100644 --- a/apps/webapp/app/routes/magic.tsx +++ b/apps/webapp/app/routes/magic.tsx @@ -8,9 +8,13 @@ import { getRedirectTo } from "~/services/redirectTo.server"; import { commitSession, getSession } from "~/services/sessionStorage.server"; import { commitAuthenticatedSession } from "~/services/sessionDuration.server"; import { trackAndClearReferralSource } from "~/services/referralSource.server"; +import { sanitizeRedirectPath } from "~/utils"; export async function loader({ request }: LoaderFunctionArgs) { - const redirectTo = await getRedirectTo(request); + // Defense-in-depth: sanitize the cookie value to drop non-page paths in case + // a stale cookie from before sanitization shipped is still in the browser. + const sanitized = sanitizeRedirectPath(await getRedirectTo(request)); + const redirectTo = sanitized === "/" ? undefined : sanitized; const auth = await authenticator.authenticate("email-link", request, { failureRedirect: "/login/magic", // If auth fails, the failureRedirect will be thrown as a Response diff --git a/apps/webapp/app/services/session.server.ts b/apps/webapp/app/services/session.server.ts index e792e4caa51..2bc7dc48adc 100644 --- a/apps/webapp/app/services/session.server.ts +++ b/apps/webapp/app/services/session.server.ts @@ -1,5 +1,6 @@ import { redirect } from "@remix-run/node"; import { getUserById } from "~/models/user.server"; +import { sanitizeRedirectPath } from "~/utils"; import { authenticator } from "./auth.server"; import { getImpersonationId } from "./impersonation.server"; import { getEffectiveSessionDuration, isSessionExpired } from "./sessionDuration.server"; @@ -50,9 +51,11 @@ export async function requireUserId(request: Request, redirectTo?: string) { const userId = await getUserId(request); if (!userId) { const url = new URL(request.url); - const searchParams = new URLSearchParams([ - ["redirectTo", redirectTo ?? `${url.pathname}${url.search}`], - ]); + // Only propagate the originating URL when it's a real user-navigable page. + // Fetcher endpoints (e.g. /resources/*) and auth callbacks would render + // blank or loop if used as a post-login destination. + const finalRedirectTo = sanitizeRedirectPath(redirectTo ?? `${url.pathname}${url.search}`); + const searchParams = new URLSearchParams([["redirectTo", finalRedirectTo]]); throw redirect(`/login?${searchParams}`); } return userId; diff --git a/apps/webapp/app/utils.ts b/apps/webapp/app/utils.ts index adb18292811..b376da97da1 100644 --- a/apps/webapp/app/utils.ts +++ b/apps/webapp/app/utils.ts @@ -3,10 +3,22 @@ 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). +const NON_NAVIGABLE_PREFIXES = ["/resources/", "/auth/", "/admin/", "/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. + * 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. */ @@ -28,16 +40,21 @@ export function sanitizeRedirectPath( return defaultRedirect; } catch {} + let parsed: URL; try { // ensure it's a valid relative path - const url = new URL(path, "https://example.com"); - if (url.hostname !== "example.com") { + 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; } From a7191180c8d85e71acd29dc78a40b019dbfbe00c Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 17:52:14 +0100 Subject: [PATCH 06/17] Fixes bug where session logout returned error. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: redirectWithErrorMessage is async, but the call site was missing await — so throw was throwing a Promise instead of a Response. Remix navigation paths happened to coerce it, but fetcher loaders (like the /resources/platform-changelogs poll) blew up with a 500. Adding await fixes both. --- apps/webapp/app/services/session.server.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/webapp/app/services/session.server.ts b/apps/webapp/app/services/session.server.ts index 2bc7dc48adc..71c435b7fda 100644 --- a/apps/webapp/app/services/session.server.ts +++ b/apps/webapp/app/services/session.server.ts @@ -1,4 +1,5 @@ import { redirect } from "@remix-run/node"; +import { redirectWithErrorMessage } from "~/models/message.server"; import { getUserById } from "~/models/user.server"; import { sanitizeRedirectPath } from "~/utils"; import { authenticator } from "./auth.server"; @@ -31,7 +32,12 @@ export async function getUserId(request: Request): Promise { const session = await getUserSession(request); const { durationSeconds } = await getEffectiveSessionDuration(authUser.userId); if (isSessionExpired(session, durationSeconds)) { - throw await logout(request); + throw await redirectWithErrorMessage( + "/logout", + request, + "You were signed out due to inactivity.", + { ephemeral: false } + ); } return authUser.userId; From fb57aa661aaf6aff102d4803e47d61f97ab6d347 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 18:44:51 +0100 Subject: [PATCH 07/17] Remove login page toast message logic --- apps/webapp/app/services/session.server.ts | 8 +----- .../app/services/sessionDuration.server.ts | 28 +++++++++++-------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/apps/webapp/app/services/session.server.ts b/apps/webapp/app/services/session.server.ts index 71c435b7fda..d73d6c249cc 100644 --- a/apps/webapp/app/services/session.server.ts +++ b/apps/webapp/app/services/session.server.ts @@ -1,5 +1,4 @@ import { redirect } from "@remix-run/node"; -import { redirectWithErrorMessage } from "~/models/message.server"; import { getUserById } from "~/models/user.server"; import { sanitizeRedirectPath } from "~/utils"; import { authenticator } from "./auth.server"; @@ -32,12 +31,7 @@ export async function getUserId(request: Request): Promise { const session = await getUserSession(request); const { durationSeconds } = await getEffectiveSessionDuration(authUser.userId); if (isSessionExpired(session, durationSeconds)) { - throw await redirectWithErrorMessage( - "/logout", - request, - "You were signed out due to inactivity.", - { ephemeral: false } - ); + throw redirect("/logout"); } return authUser.userId; diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index 7afbd60db8b..d673c6adc01 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -156,32 +156,38 @@ export function ensureSessionIssuedAt(session: Session, now: number = Date.now() } /** - * Commits the session for an authenticated user, setting `issuedAt = now` and - * the cookie's `Max-Age` to the effective session duration. Use this at every - * login/MFA-completion point so the session window starts fresh. + * The auth cookie's `Max-Age` is intentionally long (1 year) so the cookie + * always reaches the server. Actual session expiry is enforced server-side + * via `sessionIssuedAt` against the user's effective duration. If we let the + * cookie expire client-side, the user is silently logged out without the + * "signed out due to inactivity" toast. + */ +const AUTH_COOKIE_MAX_AGE_SECONDS = DEFAULT_SESSION_DURATION_SECONDS; + +/** + * Commits the session for an authenticated user, setting `issuedAt = now`. + * Use this at every login/MFA-completion point so the session window starts + * fresh. Cookie `Max-Age` is fixed; expiry is enforced server-side. */ export async function commitAuthenticatedSession( session: Session, - userId: string, + _userId: string, now: number = Date.now() ): Promise { - const { durationSeconds } = await getEffectiveSessionDuration(userId); setSessionIssuedAt(session, now); - return commitSession(session, { maxAge: durationSeconds }); + return commitSession(session, { maxAge: AUTH_COOKIE_MAX_AGE_SECONDS }); } /** * Commits the session for an authenticated user, lazily backfilling * `issuedAt` if missing. Use on every authenticated response that already - * commits the cookie (e.g. root.tsx) so legacy cookies migrate forward and - * the browser's stored Max-Age tracks the latest effective duration. + * commits the cookie (e.g. root.tsx). */ export async function commitAuthenticatedSessionLazy( session: Session, - userId: string, + _userId: string, now: number = Date.now() ): Promise { - const { durationSeconds } = await getEffectiveSessionDuration(userId); ensureSessionIssuedAt(session, now); - return commitSession(session, { maxAge: durationSeconds }); + return commitSession(session, { maxAge: AUTH_COOKIE_MAX_AGE_SECONDS }); } From 4804b00066c7c7384ba25e9338fb918d17db9814 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 18:59:45 +0100 Subject: [PATCH 08/17] Cloudwatch picks up session logout events --- apps/webapp/app/services/session.server.ts | 23 +++++++++++++++++-- .../app/services/sessionDuration.server.ts | 6 ++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/apps/webapp/app/services/session.server.ts b/apps/webapp/app/services/session.server.ts index d73d6c249cc..ce3c470eafc 100644 --- a/apps/webapp/app/services/session.server.ts +++ b/apps/webapp/app/services/session.server.ts @@ -3,7 +3,12 @@ import { getUserById } from "~/models/user.server"; import { sanitizeRedirectPath } from "~/utils"; import { authenticator } from "./auth.server"; import { getImpersonationId } from "./impersonation.server"; -import { getEffectiveSessionDuration, isSessionExpired } from "./sessionDuration.server"; +import { logger } from "./logger.server"; +import { + getEffectiveSessionDuration, + getSessionIssuedAt, + isSessionExpired, +} from "./sessionDuration.server"; import { getUserSession } from "./sessionStorage.server"; export async function getUserId(request: Request): Promise { @@ -29,8 +34,22 @@ export async function getUserId(request: Request): Promise { // by the most restrictive Organization.maxSessionDuration). If the session // was issued longer ago than the cap allows, force a logout. const session = await getUserSession(request); - const { durationSeconds } = await getEffectiveSessionDuration(authUser.userId); + const { durationSeconds, orgCapSeconds, userSettingSeconds } = await getEffectiveSessionDuration( + authUser.userId + ); if (isSessionExpired(session, durationSeconds)) { + const issuedAt = getSessionIssuedAt(session); + // HIPAA audit trail: structured log lands in CloudWatch via stdout. Use + // the stable `event` field to filter/aggregate auto-logout events. + logger.info("Auto-logout: session exceeded effective duration", { + event: "session.auto_logout", + userId: authUser.userId, + effectiveDurationSeconds: durationSeconds, + userSettingSeconds, + orgCapSeconds, + sessionAgeMs: issuedAt === null ? null : Date.now() - issuedAt, + requestPath: new URL(request.url).pathname, + }); throw redirect("/logout"); } diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index d673c6adc01..b709c500414 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -119,7 +119,7 @@ export function getAllowedSessionOptions( return allowed; } -function readSessionIssuedAt(session: Session): number | null { +export function getSessionIssuedAt(session: Session): number | null { const raw = session.get(SESSION_ISSUED_AT_KEY); if (typeof raw !== "number" || !Number.isFinite(raw)) return null; return raw; @@ -135,7 +135,7 @@ export function isSessionExpired( effectiveDurationSeconds: number, now: number = Date.now() ): boolean { - const issuedAt = readSessionIssuedAt(session); + const issuedAt = getSessionIssuedAt(session); if (issuedAt === null) return false; return now - issuedAt > effectiveDurationSeconds * 1000; } @@ -150,7 +150,7 @@ export function setSessionIssuedAt(session: Session, now: number = Date.now()): * caller knows to commit the cookie. Returns false when nothing changed. */ export function ensureSessionIssuedAt(session: Session, now: number = Date.now()): boolean { - if (readSessionIssuedAt(session) !== null) return false; + if (getSessionIssuedAt(session) !== null) return false; setSessionIssuedAt(session, now); return true; } From 5a8b8882694f4c0b5b3f54378da4a72ffe58c2bb Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 18:59:55 +0100 Subject: [PATCH 09/17] small classname tweak --- apps/webapp/app/routes/account.security/route.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/webapp/app/routes/account.security/route.tsx b/apps/webapp/app/routes/account.security/route.tsx index 02ce3f9d776..297a30b9d21 100644 --- a/apps/webapp/app/routes/account.security/route.tsx +++ b/apps/webapp/app/routes/account.security/route.tsx @@ -59,7 +59,7 @@ export default function Page() { - +
Security
From 96730fa16c442f171e461e507bf527d9c4143409 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 19:07:49 +0100 Subject: [PATCH 10/17] Aggregate the session length values --- .../app/services/sessionDuration.server.ts | 30 ++++++++----------- .../app/services/sessionStorage.server.ts | 13 ++++---- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index b709c500414..441a75ca234 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -1,18 +1,17 @@ import type { Session } from "@remix-run/node"; import type { PrismaClientOrTransaction } from "@trigger.dev/database"; import { prisma } from "~/db.server"; -import { commitSession } from "./sessionStorage.server"; +import { commitSession, DEFAULT_SESSION_DURATION_SECONDS } from "./sessionStorage.server"; + +export { DEFAULT_SESSION_DURATION_SECONDS }; export const SESSION_ISSUED_AT_KEY = "session:issuedAt"; // 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_YEAR_SECONDS = 31_556_952; // 365.2425 * 86400 const GREGORIAN_HALF_YEAR_SECONDS = 15_778_476; -export const DEFAULT_SESSION_DURATION_SECONDS = GREGORIAN_YEAR_SECONDS; - export type SessionDurationOption = { value: number; label: string; @@ -25,7 +24,7 @@ export const SESSION_DURATION_OPTIONS: SessionDurationOption[] = [ { value: 60 * 60 * 24, label: "1 day" }, { value: 60 * 60 * 24 * 30, label: "30 days" }, { value: GREGORIAN_HALF_YEAR_SECONDS, label: "6 months" }, - { value: GREGORIAN_YEAR_SECONDS, label: "1 year" }, + { value: DEFAULT_SESSION_DURATION_SECONDS, label: "1 year" }, ]; export const ALLOWED_SESSION_DURATION_VALUES: ReadonlySet = new Set( @@ -155,19 +154,16 @@ export function ensureSessionIssuedAt(session: Session, now: number = Date.now() return true; } -/** - * The auth cookie's `Max-Age` is intentionally long (1 year) so the cookie - * always reaches the server. Actual session expiry is enforced server-side - * via `sessionIssuedAt` against the user's effective duration. If we let the - * cookie expire client-side, the user is silently logged out without the - * "signed out due to inactivity" toast. - */ -const AUTH_COOKIE_MAX_AGE_SECONDS = DEFAULT_SESSION_DURATION_SECONDS; - /** * Commits the session for an authenticated user, setting `issuedAt = now`. * Use this at every login/MFA-completion point so the session window starts - * fresh. Cookie `Max-Age` is fixed; expiry is enforced server-side. + * fresh. + * + * 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 via + * `sessionIssuedAt` against the user's effective duration. If we let the + * cookie expire client-side, the user is silently logged out. */ export async function commitAuthenticatedSession( session: Session, @@ -175,7 +171,7 @@ export async function commitAuthenticatedSession( now: number = Date.now() ): Promise { setSessionIssuedAt(session, now); - return commitSession(session, { maxAge: AUTH_COOKIE_MAX_AGE_SECONDS }); + return commitSession(session, { maxAge: DEFAULT_SESSION_DURATION_SECONDS }); } /** @@ -189,5 +185,5 @@ export async function commitAuthenticatedSessionLazy( now: number = Date.now() ): Promise { ensureSessionIssuedAt(session, now); - return commitSession(session, { maxAge: AUTH_COOKIE_MAX_AGE_SECONDS }); + return commitSession(session, { maxAge: DEFAULT_SESSION_DURATION_SECONDS }); } diff --git a/apps/webapp/app/services/sessionStorage.server.ts b/apps/webapp/app/services/sessionStorage.server.ts index 90e7d069abb..c54561d647b 100644 --- a/apps/webapp/app/services/sessionStorage.server.ts +++ b/apps/webapp/app/services/sessionStorage.server.ts @@ -1,11 +1,12 @@ import { createCookieSessionStorage } from "@remix-run/node"; import { env } from "~/env.server"; -// Hard ceiling for the cookie lifetime. The actual per-session value is set -// per-commit via commitSession(session, { maxAge }) in the auth/login flows -// and on every authenticated response, derived from the user's effective -// session duration (User.sessionDuration capped by Organization.maxSessionDuration). -export const SESSION_STORAGE_MAX_AGE_SECONDS = 60 * 60 * 24 * 365; +// Canonical "1 year in seconds", using Gregorian calendar conversion +// (365.2425 * 86400) so it matches the labeled "1 year" dropdown option in +// SESSION_DURATION_OPTIONS exactly. This is the cookie's hard upper-bound +// lifetime; the actual per-session value is enforced server-side via +// `sessionIssuedAt` against the user's effective duration. +export const DEFAULT_SESSION_DURATION_SECONDS = 31_556_952; export const sessionStorage = createCookieSessionStorage({ cookie: { @@ -15,7 +16,7 @@ export const sessionStorage = createCookieSessionStorage({ httpOnly: true, secrets: [env.SESSION_SECRET], secure: env.NODE_ENV === "production", - maxAge: SESSION_STORAGE_MAX_AGE_SECONDS, + maxAge: DEFAULT_SESSION_DURATION_SECONDS, }, }); From 36ef44fa80b9e2df8234d72bc28911f08efc8e59 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 19:09:24 +0100 Subject: [PATCH 11/17] Merge the 2 db migrations --- .../20260428140746_add_session_duration_columns/migration.sql | 2 +- .../migration.sql | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) delete mode 100644 internal-packages/database/prisma/migrations/20260428152919_use_gregorian_year_for_user_session_duration_default/migration.sql diff --git a/internal-packages/database/prisma/migrations/20260428140746_add_session_duration_columns/migration.sql b/internal-packages/database/prisma/migrations/20260428140746_add_session_duration_columns/migration.sql index cf980e63e9e..f63a5d6d244 100644 --- a/internal-packages/database/prisma/migrations/20260428140746_add_session_duration_columns/migration.sql +++ b/internal-packages/database/prisma/migrations/20260428140746_add_session_duration_columns/migration.sql @@ -1,5 +1,5 @@ -- AlterTable -ALTER TABLE "public"."User" ADD COLUMN "sessionDuration" INTEGER NOT NULL DEFAULT 31536000; +ALTER TABLE "public"."User" ADD COLUMN "sessionDuration" INTEGER NOT NULL DEFAULT 31556952; -- AlterTable ALTER TABLE "public"."Organization" ADD COLUMN "maxSessionDuration" INTEGER; diff --git a/internal-packages/database/prisma/migrations/20260428152919_use_gregorian_year_for_user_session_duration_default/migration.sql b/internal-packages/database/prisma/migrations/20260428152919_use_gregorian_year_for_user_session_duration_default/migration.sql deleted file mode 100644 index 83cdb946158..00000000000 --- a/internal-packages/database/prisma/migrations/20260428152919_use_gregorian_year_for_user_session_duration_default/migration.sql +++ /dev/null @@ -1,2 +0,0 @@ --- AlterTable -ALTER TABLE "public"."User" ALTER COLUMN "sessionDuration" SET DEFAULT 31556952; From 287f5663ae4d868321b13ad62a431b0b1c7216c9 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Tue, 28 Apr 2026 19:18:51 +0100 Subject: [PATCH 12/17] userId param is no longer used --- apps/webapp/app/root.tsx | 2 +- apps/webapp/app/routes/auth.github.callback.tsx | 2 +- apps/webapp/app/routes/auth.google.callback.tsx | 2 +- apps/webapp/app/routes/login.mfa/route.tsx | 2 +- apps/webapp/app/routes/magic.tsx | 2 +- .../app/routes/resources.account.session-duration/route.tsx | 2 +- apps/webapp/app/services/sessionDuration.server.ts | 2 -- 7 files changed, 6 insertions(+), 8 deletions(-) diff --git a/apps/webapp/app/root.tsx b/apps/webapp/app/root.tsx index bf62104b64a..be48826ccc2 100644 --- a/apps/webapp/app/root.tsx +++ b/apps/webapp/app/root.tsx @@ -70,7 +70,7 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { // current effective session duration. if (user) { const authSession = await getUserSession(request); - headers.append("Set-Cookie", await commitAuthenticatedSessionLazy(authSession, user.id)); + headers.append("Set-Cookie", await commitAuthenticatedSessionLazy(authSession)); } return typedjson( diff --git a/apps/webapp/app/routes/auth.github.callback.tsx b/apps/webapp/app/routes/auth.github.callback.tsx index 4feaeb0d843..02bf67674b3 100644 --- a/apps/webapp/app/routes/auth.github.callback.tsx +++ b/apps/webapp/app/routes/auth.github.callback.tsx @@ -53,7 +53,7 @@ export let loader: LoaderFunction = async ({ request }) => { session.set(authenticator.sessionKey, auth); const headers = new Headers(); - headers.append("Set-Cookie", await commitAuthenticatedSession(session, auth.userId)); + headers.append("Set-Cookie", await commitAuthenticatedSession(session)); headers.append("Set-Cookie", await setLastAuthMethodHeader("github")); await trackAndClearReferralSource(request, auth.userId, headers); diff --git a/apps/webapp/app/routes/auth.google.callback.tsx b/apps/webapp/app/routes/auth.google.callback.tsx index 735cdab9f0f..c6aefdb758c 100644 --- a/apps/webapp/app/routes/auth.google.callback.tsx +++ b/apps/webapp/app/routes/auth.google.callback.tsx @@ -53,7 +53,7 @@ export let loader: LoaderFunction = async ({ request }) => { session.set(authenticator.sessionKey, auth); const headers = new Headers(); - headers.append("Set-Cookie", await commitAuthenticatedSession(session, auth.userId)); + headers.append("Set-Cookie", await commitAuthenticatedSession(session)); headers.append("Set-Cookie", await setLastAuthMethodHeader("google")); await trackAndClearReferralSource(request, auth.userId, headers); diff --git a/apps/webapp/app/routes/login.mfa/route.tsx b/apps/webapp/app/routes/login.mfa/route.tsx index 67006c37482..c02aea0cfde 100644 --- a/apps/webapp/app/routes/login.mfa/route.tsx +++ b/apps/webapp/app/routes/login.mfa/route.tsx @@ -163,7 +163,7 @@ async function completeLogin(request: Request, session: Session, userId: string) session.unset("pending-mfa-redirect-to"); const headers = new Headers(); - headers.append("Set-Cookie", await commitAuthenticatedSession(session, userId)); + headers.append("Set-Cookie", await commitAuthenticatedSession(session)); await trackAndClearReferralSource(request, userId, headers); diff --git a/apps/webapp/app/routes/magic.tsx b/apps/webapp/app/routes/magic.tsx index d4606e1b7de..a3d677829c8 100644 --- a/apps/webapp/app/routes/magic.tsx +++ b/apps/webapp/app/routes/magic.tsx @@ -56,7 +56,7 @@ export async function loader({ request }: LoaderFunctionArgs) { session.set(authenticator.sessionKey, auth); const headers = new Headers(); - headers.append("Set-Cookie", await commitAuthenticatedSession(session, auth.userId)); + headers.append("Set-Cookie", await commitAuthenticatedSession(session)); headers.append("Set-Cookie", await setLastAuthMethodHeader("email")); await trackAndClearReferralSource(request, auth.userId, headers); diff --git a/apps/webapp/app/routes/resources.account.session-duration/route.tsx b/apps/webapp/app/routes/resources.account.session-duration/route.tsx index edf617e73fa..b57fc3cc02b 100644 --- a/apps/webapp/app/routes/resources.account.session-duration/route.tsx +++ b/apps/webapp/app/routes/resources.account.session-duration/route.tsx @@ -63,7 +63,7 @@ export async function action({ request }: ActionFunctionArgs) { // Re-issue the cookie with the new maxAge and reset issuedAt so the user // gets a fresh window matching their new selection right away. const authSession = await getUserSession(request); - const authCookie = await commitAuthenticatedSession(authSession, userId); + const authCookie = await commitAuthenticatedSession(authSession); const messageSession = await getMessageSession(request.headers.get("cookie")); setSuccessMessage(messageSession, "Session duration updated."); diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index 441a75ca234..383baf6832a 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -167,7 +167,6 @@ export function ensureSessionIssuedAt(session: Session, now: number = Date.now() */ export async function commitAuthenticatedSession( session: Session, - _userId: string, now: number = Date.now() ): Promise { setSessionIssuedAt(session, now); @@ -181,7 +180,6 @@ export async function commitAuthenticatedSession( */ export async function commitAuthenticatedSessionLazy( session: Session, - _userId: string, now: number = Date.now() ): Promise { ensureSessionIssuedAt(session, now); From 8062e16d20f0bf4a5b2da501d0e9fde29ebca732 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Wed, 29 Apr 2026 14:52:04 +0100 Subject: [PATCH 13/17] Fix for ensuring a DB change updates the availble duration options --- .../app/routes/account.security/route.tsx | 20 +++++-------------- .../route.tsx | 4 ++-- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/apps/webapp/app/routes/account.security/route.tsx b/apps/webapp/app/routes/account.security/route.tsx index 297a30b9d21..48db2e896f2 100644 --- a/apps/webapp/app/routes/account.security/route.tsx +++ b/apps/webapp/app/routes/account.security/route.tsx @@ -1,5 +1,5 @@ import { type MetaFunction } from "@remix-run/react"; -import { LoaderFunctionArgs } from "@remix-run/server-runtime"; +import type { LoaderFunctionArgs } from "@remix-run/server-runtime"; import { typedjson, useTypedLoaderData } from "remix-typedjson"; import { MainHorizontallyCenteredContainer, @@ -8,12 +8,10 @@ import { } from "~/components/layout/AppLayout"; import { Header2 } from "~/components/primitives/Headers"; import { NavBar, PageTitle } from "~/components/primitives/PageHeader"; -import { prisma } from "~/db.server"; import { requireUser } from "~/services/session.server"; import { - DEFAULT_SESSION_DURATION_SECONDS, getAllowedSessionOptions, - getOrganizationSessionCap, + getEffectiveSessionDuration, } from "~/services/sessionDuration.server"; import { MfaSetup } from "../resources.account.mfa.setup/route"; import { SessionDurationSetting } from "../resources.account.session-duration/SessionDurationSetting"; @@ -29,20 +27,12 @@ export const meta: MetaFunction = () => { export async function loader({ request }: LoaderFunctionArgs) { const user = await requireUser(request); - const [userRecord, orgCapSeconds] = await Promise.all([ - prisma.user.findUnique({ - where: { id: user.id }, - select: { sessionDuration: true }, - }), - getOrganizationSessionCap(user.id), - ]); - - const sessionDuration = userRecord?.sessionDuration ?? DEFAULT_SESSION_DURATION_SECONDS; - const sessionDurationOptions = getAllowedSessionOptions(orgCapSeconds, sessionDuration); + const { durationSeconds, orgCapSeconds } = await getEffectiveSessionDuration(user.id); + const sessionDurationOptions = getAllowedSessionOptions(orgCapSeconds, durationSeconds); return typedjson({ user, - sessionDuration, + sessionDuration: durationSeconds, sessionDurationOptions, orgCapSeconds, }); diff --git a/apps/webapp/app/routes/resources.account.session-duration/route.tsx b/apps/webapp/app/routes/resources.account.session-duration/route.tsx index b57fc3cc02b..1b114f1d4bb 100644 --- a/apps/webapp/app/routes/resources.account.session-duration/route.tsx +++ b/apps/webapp/app/routes/resources.account.session-duration/route.tsx @@ -46,8 +46,8 @@ export async function action({ request }: ActionFunctionArgs) { return redirectWithError(request, "Invalid session duration value."); } - const { orgCapSeconds, userSettingSeconds } = await getEffectiveSessionDuration(userId); - const allowed = getAllowedSessionOptions(orgCapSeconds, userSettingSeconds); + const { orgCapSeconds, durationSeconds } = await getEffectiveSessionDuration(userId); + const allowed = getAllowedSessionOptions(orgCapSeconds, durationSeconds); if (!allowed.some((o) => o.value === sessionDuration)) { return redirectWithError( request, From 2726647d5a5094ff142a841a66405db755e77f6a Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Thu, 30 Apr 2026 21:28:06 +0100 Subject: [PATCH 14/17] Code review fixes and improvements --- .../app/routes/account.security/route.tsx | 3 +- .../app/routes/auth.github.callback.tsx | 6 +- .../app/routes/auth.google.callback.tsx | 6 +- .../resources.account.mfa.setup/MfaToggle.tsx | 2 +- apps/webapp/app/services/session.server.ts | 72 ++++++++++++------- .../app/services/sessionDuration.server.ts | 2 +- apps/webapp/app/utils.ts | 6 +- 7 files changed, 62 insertions(+), 35 deletions(-) diff --git a/apps/webapp/app/routes/account.security/route.tsx b/apps/webapp/app/routes/account.security/route.tsx index 48db2e896f2..68dc8d1fcf4 100644 --- a/apps/webapp/app/routes/account.security/route.tsx +++ b/apps/webapp/app/routes/account.security/route.tsx @@ -8,6 +8,7 @@ import { } from "~/components/layout/AppLayout"; import { Header2 } from "~/components/primitives/Headers"; import { NavBar, PageTitle } from "~/components/primitives/PageHeader"; +import { $replica } from "~/db.server"; import { requireUser } from "~/services/session.server"; import { getAllowedSessionOptions, @@ -27,7 +28,7 @@ export const meta: MetaFunction = () => { export async function loader({ request }: LoaderFunctionArgs) { const user = await requireUser(request); - const { durationSeconds, orgCapSeconds } = await getEffectiveSessionDuration(user.id); + const { durationSeconds, orgCapSeconds } = await getEffectiveSessionDuration(user.id, $replica); const sessionDurationOptions = getAllowedSessionOptions(orgCapSeconds, durationSeconds); return typedjson({ diff --git a/apps/webapp/app/routes/auth.github.callback.tsx b/apps/webapp/app/routes/auth.github.callback.tsx index 02bf67674b3..f0c8e9a4515 100644 --- a/apps/webapp/app/routes/auth.github.callback.tsx +++ b/apps/webapp/app/routes/auth.github.callback.tsx @@ -1,10 +1,10 @@ import type { LoaderFunction } from "@remix-run/node"; import { redirect } from "@remix-run/node"; import { prisma } from "~/db.server"; -import { getSession, redirectWithErrorMessage } from "~/models/message.server"; +import { redirectWithErrorMessage } from "~/models/message.server"; import { authenticator } from "~/services/auth.server"; import { setLastAuthMethodHeader } from "~/services/lastAuthMethod.server"; -import { commitSession } from "~/services/sessionStorage.server"; +import { commitSession, getUserSession } from "~/services/sessionStorage.server"; import { commitAuthenticatedSession } from "~/services/sessionDuration.server"; import { trackAndClearReferralSource } from "~/services/referralSource.server"; import { redirectCookie } from "./auth.github"; @@ -19,7 +19,7 @@ export let loader: LoaderFunction = async ({ request }) => { failureRedirect: "/login", // If auth fails, the failureRedirect will be thrown as a Response }); - const session = await getSession(request.headers.get("cookie")); + const session = await getUserSession(request); const userRecord = await prisma.user.findFirst({ where: { diff --git a/apps/webapp/app/routes/auth.google.callback.tsx b/apps/webapp/app/routes/auth.google.callback.tsx index c6aefdb758c..732903ca0ef 100644 --- a/apps/webapp/app/routes/auth.google.callback.tsx +++ b/apps/webapp/app/routes/auth.google.callback.tsx @@ -1,10 +1,10 @@ import type { LoaderFunction } from "@remix-run/node"; import { redirect } from "@remix-run/node"; import { prisma } from "~/db.server"; -import { getSession, redirectWithErrorMessage } from "~/models/message.server"; +import { redirectWithErrorMessage } from "~/models/message.server"; import { authenticator } from "~/services/auth.server"; import { setLastAuthMethodHeader } from "~/services/lastAuthMethod.server"; -import { commitSession } from "~/services/sessionStorage.server"; +import { commitSession, getUserSession } from "~/services/sessionStorage.server"; import { commitAuthenticatedSession } from "~/services/sessionDuration.server"; import { trackAndClearReferralSource } from "~/services/referralSource.server"; import { redirectCookie } from "./auth.google"; @@ -19,7 +19,7 @@ export let loader: LoaderFunction = async ({ request }) => { failureRedirect: "/login", // If auth fails, the failureRedirect will be thrown as a Response }); - const session = await getSession(request.headers.get("cookie")); + const session = await getUserSession(request); const userRecord = await prisma.user.findFirst({ where: { diff --git a/apps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsx b/apps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsx index de652fae075..c63c2eb8f84 100644 --- a/apps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsx +++ b/apps/webapp/app/routes/resources.account.mfa.setup/MfaToggle.tsx @@ -14,7 +14,7 @@ export function MfaToggle({ isEnabled, onToggle }: MfaToggleProps) {
- + Require a one-time code from your authenticator app (TOTP). diff --git a/apps/webapp/app/services/session.server.ts b/apps/webapp/app/services/session.server.ts index ce3c470eafc..a603ed7dc47 100644 --- a/apps/webapp/app/services/session.server.ts +++ b/apps/webapp/app/services/session.server.ts @@ -1,4 +1,5 @@ import { redirect } from "@remix-run/node"; +import { $replica } from "~/db.server"; import { getUserById } from "~/models/user.server"; import { sanitizeRedirectPath } from "~/utils"; import { authenticator } from "./auth.server"; @@ -11,6 +12,44 @@ import { } from "./sessionDuration.server"; import { getUserSession } from "./sessionStorage.server"; +/** + * Enforces the user's effective session duration (User.sessionDuration capped + * by the most restrictive Organization.maxSessionDuration). If the session was + * issued longer ago than the cap allows, throws a redirect to `/logout` and + * emits a HIPAA audit log. `userId` is always the *session owner's* id (i.e. + * the real authenticated user), not an impersonated one — because the cap + * belongs to the cookie, not the impersonation target. + */ +async function enforceSessionExpiry( + request: Request, + userId: string, + impersonatedUserId: string | null = null +): Promise { + const session = await getUserSession(request); + // Hot path: every authenticated request runs this. Read from the replica + // when one is configured (falls back to primary). Stale-by-replica-lag is + // acceptable here because the worst case is a session living a few seconds + // past its cap on the very first request after a cap change. + const { durationSeconds, orgCapSeconds, userSettingSeconds } = + await getEffectiveSessionDuration(userId, $replica); + if (!isSessionExpired(session, durationSeconds)) return; + + const issuedAt = getSessionIssuedAt(session); + // HIPAA audit trail: structured log lands in CloudWatch via stdout. Use + // the stable `event` field to filter/aggregate auto-logout events. + logger.info("Auto-logout: session exceeded effective duration", { + event: "session.auto_logout", + userId, + impersonatedUserId, + effectiveDurationSeconds: durationSeconds, + userSettingSeconds, + orgCapSeconds, + sessionAgeMs: issuedAt === null ? null : Date.now() - issuedAt, + requestPath: new URL(request.url).pathname, + }); + throw redirect("/logout"); +} + export async function getUserId(request: Request): Promise { const impersonatedUserId = await getImpersonationId(request); @@ -20,39 +59,24 @@ export async function getUserId(request: Request): Promise { if (authUser?.userId) { const realUser = await getUserById(authUser.userId); if (realUser?.admin) { + // Enforce expiry against the admin's own session — impersonation must + // not be a way to bypass the admin's effective duration cap. + await enforceSessionExpiry(request, authUser.userId, impersonatedUserId); return impersonatedUserId; } } - // Admin revoked or session invalid — fall through to return the real user's ID + // Admin revoked or session invalid — fall through to return the real + // user's ID. Same enforcement as the regular auth path below. + if (authUser?.userId) { + await enforceSessionExpiry(request, authUser.userId); + } return authUser?.userId; } const authUser = await authenticator.isAuthenticated(request); if (!authUser?.userId) return undefined; - // Enforce the user's effective session duration (User.sessionDuration capped - // by the most restrictive Organization.maxSessionDuration). If the session - // was issued longer ago than the cap allows, force a logout. - const session = await getUserSession(request); - const { durationSeconds, orgCapSeconds, userSettingSeconds } = await getEffectiveSessionDuration( - authUser.userId - ); - if (isSessionExpired(session, durationSeconds)) { - const issuedAt = getSessionIssuedAt(session); - // HIPAA audit trail: structured log lands in CloudWatch via stdout. Use - // the stable `event` field to filter/aggregate auto-logout events. - logger.info("Auto-logout: session exceeded effective duration", { - event: "session.auto_logout", - userId: authUser.userId, - effectiveDurationSeconds: durationSeconds, - userSettingSeconds, - orgCapSeconds, - sessionAgeMs: issuedAt === null ? null : Date.now() - issuedAt, - requestPath: new URL(request.url).pathname, - }); - throw redirect("/logout"); - } - + await enforceSessionExpiry(request, authUser.userId); return authUser.userId; } diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index 383baf6832a..7ffbb917c34 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -74,7 +74,7 @@ export async function getEffectiveSessionDuration( client: PrismaClientOrTransaction = prisma ): Promise { const [user, orgCap] = await Promise.all([ - client.user.findUnique({ + client.user.findFirst({ where: { id: userId }, select: { sessionDuration: true }, }), diff --git a/apps/webapp/app/utils.ts b/apps/webapp/app/utils.ts index b376da97da1..7551bef1b6f 100644 --- a/apps/webapp/app/utils.ts +++ b/apps/webapp/app/utils.ts @@ -5,8 +5,10 @@ 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). -const NON_NAVIGABLE_PREFIXES = ["/resources/", "/auth/", "/admin/", "/api/", "/engine/"]; +// 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 { From de992dd82cfb3463bef09d3333594a61f362dd1b Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Thu, 30 Apr 2026 22:14:07 +0100 Subject: [PATCH 15/17] Code review improvements --- ...1.orgs.$organizationId.session-duration.ts | 26 ++++++++++++++-- apps/webapp/app/services/session.server.ts | 7 ++++- .../app/services/sessionDuration.server.ts | 30 +++++++++++++------ apps/webapp/test/sessionDuration.test.ts | 13 ++++---- 4 files changed, 59 insertions(+), 17 deletions(-) diff --git a/apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.session-duration.ts b/apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.session-duration.ts index 55090a1c1f1..5da26918693 100644 --- a/apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.session-duration.ts +++ b/apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.session-duration.ts @@ -2,6 +2,10 @@ import { type ActionFunctionArgs, json } from "@remix-run/server-runtime"; import { z } from "zod"; import { prisma } from "~/db.server"; import { requireAdminApiRequest } from "~/services/personalAccessToken.server"; +import { + ALLOWED_SESSION_DURATION_VALUES, + isAllowedSessionDuration, +} from "~/services/sessionDuration.server"; const ParamsSchema = z.object({ organizationId: z.string(), @@ -12,15 +16,33 @@ const RequestBodySchema = z.object({ * Maximum session lifetime (seconds) for members of this organization, or * null to remove the cap. When set, this caps each member's * `User.sessionDuration` and is enforced on the user's next request. + * + * Must be one of the values in `SESSION_DURATION_OPTIONS` so the cap always + * maps to a labeled dropdown option for users — otherwise users see fallback + * labels like "7200 seconds" in the UI. To allow a new value, add it to + * `SESSION_DURATION_OPTIONS`. */ - maxSessionDuration: z.number().int().positive().nullable(), + maxSessionDuration: z + .number() + .int() + .positive() + .nullable() + .refine((v) => v === null || isAllowedSessionDuration(v), { + message: `maxSessionDuration must be one of: ${[...ALLOWED_SESSION_DURATION_VALUES] + .sort((a, b) => a - b) + .join(", ")}`, + }), }); export async function action({ request, params }: ActionFunctionArgs) { await requireAdminApiRequest(request); const { organizationId } = ParamsSchema.parse(params); - const body = RequestBodySchema.parse(await request.json()); + const parseResult = RequestBodySchema.safeParse(await request.json()); + if (!parseResult.success) { + return json({ success: false, errors: parseResult.error.flatten() }, { status: 400 }); + } + const body = parseResult.data; const organization = await prisma.organization.update({ where: { id: organizationId }, diff --git a/apps/webapp/app/services/session.server.ts b/apps/webapp/app/services/session.server.ts index a603ed7dc47..c471b1f45c4 100644 --- a/apps/webapp/app/services/session.server.ts +++ b/apps/webapp/app/services/session.server.ts @@ -2,6 +2,7 @@ import { redirect } from "@remix-run/node"; import { $replica } from "~/db.server"; import { getUserById } from "~/models/user.server"; import { sanitizeRedirectPath } from "~/utils"; +import { extractClientIp } from "~/utils/extractClientIp.server"; import { authenticator } from "./auth.server"; import { getImpersonationId } from "./impersonation.server"; import { logger } from "./logger.server"; @@ -30,22 +31,26 @@ async function enforceSessionExpiry( // when one is configured (falls back to primary). Stale-by-replica-lag is // acceptable here because the worst case is a session living a few seconds // past its cap on the very first request after a cap change. - const { durationSeconds, orgCapSeconds, userSettingSeconds } = + const { durationSeconds, orgCapSeconds, cappingOrgId, userSettingSeconds } = await getEffectiveSessionDuration(userId, $replica); if (!isSessionExpired(session, durationSeconds)) return; const issuedAt = getSessionIssuedAt(session); // HIPAA audit trail: structured log lands in CloudWatch via stdout. Use // the stable `event` field to filter/aggregate auto-logout events. + // `sourceIp` uses ALB's appended (last) X-Forwarded-For element, not the + // first one, since the leading element is client-supplied and spoofable. logger.info("Auto-logout: session exceeded effective duration", { event: "session.auto_logout", userId, impersonatedUserId, + cappingOrgId, effectiveDurationSeconds: durationSeconds, userSettingSeconds, orgCapSeconds, sessionAgeMs: issuedAt === null ? null : Date.now() - issuedAt, requestPath: new URL(request.url).pathname, + sourceIp: extractClientIp(request.headers.get("x-forwarded-for")), }); throw redirect("/logout"); } diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index 7ffbb917c34..98cdae59ba0 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -35,24 +35,33 @@ 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 (in seconds) across all of - * the user's organizations, ignoring orgs where it's null. Returns null when - * no org has set a cap. + * 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 { - const result = await client.organization.aggregate({ +): Promise { + const tightest = await client.organization.findFirst({ where: { members: { some: { userId } }, maxSessionDuration: { not: null }, deletedAt: null, }, - _min: { maxSessionDuration: true }, + orderBy: { maxSessionDuration: "asc" }, + select: { id: true, maxSessionDuration: true }, }); - return result._min.maxSessionDuration ?? null; + if (!tightest || tightest.maxSessionDuration === null) return null; + return { orgCapSeconds: tightest.maxSessionDuration, cappingOrgId: tightest.id }; } export type EffectiveSessionDuration = { @@ -60,6 +69,8 @@ export type EffectiveSessionDuration = { 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; }; @@ -83,11 +94,12 @@ export async function getEffectiveSessionDuration( const userSettingSeconds = user?.sessionDuration ?? DEFAULT_SESSION_DURATION_SECONDS; const durationSeconds = - orgCap === null ? userSettingSeconds : Math.min(userSettingSeconds, orgCap); + orgCap === null ? userSettingSeconds : Math.min(userSettingSeconds, orgCap.orgCapSeconds); return { durationSeconds, - orgCapSeconds: orgCap, + orgCapSeconds: orgCap?.orgCapSeconds ?? null, + cappingOrgId: orgCap?.cappingOrgId ?? null, userSettingSeconds, }; } diff --git a/apps/webapp/test/sessionDuration.test.ts b/apps/webapp/test/sessionDuration.test.ts index 17b4b932c7e..5f6b9ab93c7 100644 --- a/apps/webapp/test/sessionDuration.test.ts +++ b/apps/webapp/test/sessionDuration.test.ts @@ -145,18 +145,18 @@ describe("getOrganizationSessionCap", () => { async ({ prisma }) => { const user = await createUser(prisma, "multi-org@test.com"); await createOrgWithMember(prisma, "loose-org", user.id, oneDay); - await createOrgWithMember(prisma, "tight-org", user.id, oneHour); + const tight = await createOrgWithMember(prisma, "tight-org", user.id, oneHour); await createOrgWithMember(prisma, "uncapped-org", user.id, null); const cap = await getOrganizationSessionCap(user.id, prisma); - expect(cap).toBe(oneHour); + expect(cap).toEqual({ orgCapSeconds: oneHour, cappingOrgId: tight.id }); } ); containerTest("ignores soft-deleted organizations", async ({ prisma }) => { const user = await createUser(prisma, "deleted-org-user@test.com"); const tight = await createOrgWithMember(prisma, "deleted-tight", user.id, oneHour); - await createOrgWithMember(prisma, "active-loose", user.id, oneDay); + const loose = await createOrgWithMember(prisma, "active-loose", user.id, oneDay); await prisma.organization.update({ where: { id: tight.id }, @@ -164,7 +164,7 @@ describe("getOrganizationSessionCap", () => { }); const cap = await getOrganizationSessionCap(user.id, prisma); - expect(cap).toBe(oneDay); + expect(cap).toEqual({ orgCapSeconds: oneDay, cappingOrgId: loose.id }); }); }); @@ -178,17 +178,19 @@ describe("getEffectiveSessionDuration", () => { const result = await getEffectiveSessionDuration(user.id, prisma); expect(result.userSettingSeconds).toBe(oneDay); expect(result.orgCapSeconds).toBeNull(); + expect(result.cappingOrgId).toBeNull(); expect(result.durationSeconds).toBe(oneDay); } ); containerTest("caps the user setting at the most restrictive org cap", async ({ prisma }) => { const user = await createUser(prisma, "effective-capped@test.com", oneYear); - await createOrgWithMember(prisma, "effective-capped-org", user.id, oneHour); + const org = await createOrgWithMember(prisma, "effective-capped-org", user.id, oneHour); const result = await getEffectiveSessionDuration(user.id, prisma); expect(result.userSettingSeconds).toBe(oneYear); expect(result.orgCapSeconds).toBe(oneHour); + expect(result.cappingOrgId).toBe(org.id); expect(result.durationSeconds).toBe(oneHour); }); @@ -209,6 +211,7 @@ describe("getEffectiveSessionDuration", () => { const result = await getEffectiveSessionDuration("nonexistent-user-id", prisma); expect(result.userSettingSeconds).toBe(DEFAULT_SESSION_DURATION_SECONDS); expect(result.orgCapSeconds).toBeNull(); + expect(result.cappingOrgId).toBeNull(); expect(result.durationSeconds).toBe(DEFAULT_SESSION_DURATION_SECONDS); } ); From 9faf77fc40f80103c72d433267302344c7177c58 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Thu, 30 Apr 2026 23:07:01 +0100 Subject: [PATCH 16/17] Code review fix --- apps/webapp/app/root.tsx | 7 ++++--- apps/webapp/app/services/sessionDuration.server.ts | 13 ++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/apps/webapp/app/root.tsx b/apps/webapp/app/root.tsx index be48826ccc2..a17a4159bd1 100644 --- a/apps/webapp/app/root.tsx +++ b/apps/webapp/app/root.tsx @@ -66,11 +66,12 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { headers.append("Set-Cookie", await commitSession(session)); // Lazy-backfill the auth session's `issuedAt` for cookies issued before this - // feature shipped, and refresh the cookie's Max-Age to track the user's - // current effective session duration. + // feature shipped. Returns null (and does not commit) once issuedAt is set, + // so the cookie isn't re-written on every page load. if (user) { const authSession = await getUserSession(request); - headers.append("Set-Cookie", await commitAuthenticatedSessionLazy(authSession)); + const lazyCookie = await commitAuthenticatedSessionLazy(authSession); + if (lazyCookie) headers.append("Set-Cookie", lazyCookie); } return typedjson( diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index 98cdae59ba0..1364e4aef35 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -186,14 +186,17 @@ export async function commitAuthenticatedSession( } /** - * Commits the session for an authenticated user, lazily backfilling - * `issuedAt` if missing. Use on every authenticated response that already - * commits the cookie (e.g. root.tsx). + * Lazily backfills `issuedAt` on legacy auth sessions that predate the + * sessionDuration feature. Returns the cookie string when a backfill happened + * (caller must append it to the response's `Set-Cookie` headers), or `null` + * when the session already had `issuedAt` set — avoiding an unnecessary + * Set-Cookie on every authenticated page load and preventing the cookie's + * 1-year Max-Age from rolling forward indefinitely. */ export async function commitAuthenticatedSessionLazy( session: Session, now: number = Date.now() -): Promise { - ensureSessionIssuedAt(session, now); +): Promise { + if (!ensureSessionIssuedAt(session, now)) return null; return commitSession(session, { maxAge: DEFAULT_SESSION_DURATION_SECONDS }); } From b768149faed01c8e30990a150d9586131a6461f6 Mon Sep 17 00:00:00 2001 From: James Ritchie Date: Thu, 30 Apr 2026 23:19:44 +0100 Subject: [PATCH 17/17] code comment update --- .../app/services/sessionDuration.server.ts | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/services/sessionDuration.server.ts b/apps/webapp/app/services/sessionDuration.server.ts index 1364e4aef35..da1636ebba3 100644 --- a/apps/webapp/app/services/sessionDuration.server.ts +++ b/apps/webapp/app/services/sessionDuration.server.ts @@ -105,10 +105,22 @@ export async function getEffectiveSessionDuration( } /** - * Returns the dropdown options the user is allowed to pick. If an org cap - * exists, options strictly greater than the cap are removed. The user's - * currently-saved value is always included even if it now exceeds the cap, so - * the form remains valid until they pick a smaller value. + * 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,