diff --git a/server/middlewares/authentication.test.ts b/server/middlewares/authentication.test.ts index 0ecb2a37f19d..6ffd0482da8c 100644 --- a/server/middlewares/authentication.test.ts +++ b/server/middlewares/authentication.test.ts @@ -11,6 +11,7 @@ import { } from "@server/test/factories"; import { User } from "@server/models"; import { AuthenticationType } from "@server/types"; +import { JWT_COOKIE_TTL_DAYS } from "@server/utils/authentication"; import auth, { FORWARDAUTH_SERVICE } from "./authentication"; function createCtx(overrides: any = {}) { @@ -379,6 +380,47 @@ describe("Authentication middleware", () => { expect(state.auth.type).toEqual(AuthenticationType.APP); }); + it("should issue the accessToken cookie with a 7-day expiry", async () => { + const team = await buildTeam(); + const user = await buildUser({ teamId: team.id }); + const state = {} as DefaultState; + const authMiddleware = auth(); + const cookiesSet = jest.fn(); + const before = Date.now(); + + await authMiddleware( + { + // @ts-expect-error mock request + request: { + get: jest.fn((header: string) => { + if (header === "x-auth-request-email") { + return user.email!; + } + return ""; + }), + }, + // @ts-expect-error mock cookies + cookies: { get: jest.fn(() => undefined), set: cookiesSet }, + state, + ip: "127.0.0.1", + cache: {}, + }, + jest.fn() + ); + + const accessTokenCall = cookiesSet.mock.calls.find( + (call) => call[0] === "accessToken" + ); + expect(accessTokenCall).toBeDefined(); + + const expires: Date = accessTokenCall![2].expires; + const ageMs = expires.getTime() - before; + const expectedMs = JWT_COOKIE_TTL_DAYS * 24 * 60 * 60 * 1000; + // Allow ±60s skew for test runtime. + expect(ageMs).toBeGreaterThan(expectedMs - 60_000); + expect(ageMs).toBeLessThan(expectedMs + 60_000); + }); + it("should provision a new user when X-Auth-Request-Email is unknown", async () => { await buildTeam(); const state = {} as DefaultState; diff --git a/server/middlewares/authentication.ts b/server/middlewares/authentication.ts index 2160e6008d09..83bd7ec2787d 100644 --- a/server/middlewares/authentication.ts +++ b/server/middlewares/authentication.ts @@ -1,4 +1,4 @@ -import { addMonths } from "date-fns"; +import { addDays } from "date-fns"; import type { Next } from "koa"; import capitalize from "lodash/capitalize"; import { Op } from "sequelize"; @@ -18,6 +18,7 @@ import { User, Team, ApiKey, OAuthAuthentication } from "@server/models"; import { sequelize } from "@server/storage/database"; import type { AppContext } from "@server/types"; import { AuthenticationType } from "@server/types"; +import { JWT_COOKIE_TTL_DAYS } from "@server/utils/authentication"; import { getUserForJWT } from "@server/utils/jwt"; import { AuthenticationError, @@ -56,7 +57,7 @@ export default function auth(options: AuthenticationOptions = {}) { // that subsequent requests and cookie-dependent services (WebSocket, // collaboration) use the fast JWT path instead of the header DB path. if (service === FORWARDAUTH_SERVICE && !ctx.cookies.get("accessToken")) { - const expires = addMonths(new Date(), 3); + const expires = addDays(new Date(), JWT_COOKIE_TTL_DAYS); ctx.cookies.set("accessToken", user.getJwtToken(expires, service), { sameSite: "lax", expires, diff --git a/server/routes/auth/index.test.ts b/server/routes/auth/index.test.ts index 61bd5f308b04..9f29fc5a97d1 100644 --- a/server/routes/auth/index.test.ts +++ b/server/routes/auth/index.test.ts @@ -1,5 +1,6 @@ import { buildUser, buildCollection } from "@server/test/factories"; import { getTestServer } from "@server/test/support"; +import { JWT_COOKIE_TTL_DAYS } from "@server/utils/authentication"; const server = getTestServer(); @@ -43,4 +44,40 @@ describe("auth/redirect", () => { expect(res.status).toEqual(401); }); + + it("should mint an accessToken JWT carrying an expiresAt claim ~JWT_COOKIE_TTL_DAYS out", async () => { + const user = await buildUser(); + const before = Date.now(); + + const res = await server.get( + `/auth/redirect?token=${user.getTransferToken()}`, + { + redirect: "manual", + } + ); + + expect(res.status).toEqual(302); + + // Pull the `accessToken` cookie out of the Set-Cookie header(s). + const setCookie = res.headers.get("set-cookie") || ""; + const match = setCookie.match(/accessToken=([^;,]+)/); + expect(match).not.toBeNull(); + const jwt = match![1]; + + // Decode the JWT payload directly — no signature check needed, we're + // only inspecting the claim. JWT payload is the base64url middle segment. + const payload = JSON.parse( + Buffer.from(jwt.split(".")[1], "base64url").toString() + ); + + // Without the fix, getJwtToken(undefined, ...) would omit this claim + // entirely and the validator at utils/jwt.ts:47 would skip the check. + expect(payload.expiresAt).toBeDefined(); + + const ageMs = new Date(payload.expiresAt).getTime() - before; + const expectedMs = JWT_COOKIE_TTL_DAYS * 24 * 60 * 60 * 1000; + // Allow ±60s skew for test runtime. + expect(ageMs).toBeGreaterThan(expectedMs - 60_000); + expect(ageMs).toBeLessThan(expectedMs + 60_000); + }); }); diff --git a/server/routes/auth/index.ts b/server/routes/auth/index.ts index ccec003e5fbd..d0d6be6c52b1 100644 --- a/server/routes/auth/index.ts +++ b/server/routes/auth/index.ts @@ -1,15 +1,17 @@ import passport from "@outlinewiki/koa-passport"; -import { addMonths } from "date-fns"; +import { addDays } from "date-fns"; import Koa from "koa"; import bodyParser from "koa-body"; import Router from "koa-router"; import { AuthenticationError } from "@server/errors"; import authMiddleware from "@server/middlewares/authentication"; import coalesceBody from "@server/middlewares/coaleseBody"; +import { verifyCSRFToken } from "@server/middlewares/csrf"; import { Collection, Team, View } from "@server/models"; import AuthenticationHelper from "@server/models/helpers/AuthenticationHelper"; import type { AppState, AppContext, APIContext } from "@server/types"; -import { verifyCSRFToken } from "@server/middlewares/csrf"; +import { JWT_COOKIE_TTL_DAYS } from "@server/utils/authentication"; +import { getJWTPayload } from "@server/utils/jwt"; const app = new Koa(); const router = new Router(); @@ -32,18 +34,29 @@ void (async () => { router.get("/redirect", authMiddleware(), async (ctx: APIContext) => { const { user, service } = ctx.state.auth; - const jwtToken = user.getJwtToken(undefined, service); - if (jwtToken === ctx.state.auth.token) { + // This route is only for exchanging a short-lived transfer token for a + // session cookie. Reject anything else (in particular session JWTs being + // replayed to extend their own life). The previous heuristic relied on + // a quirk where session JWTs were deterministic; with proper `expiresAt` + // claims they no longer are, so check the token type directly. + if (getJWTPayload(ctx.state.auth.token).type !== "transfer") { throw AuthenticationError("Cannot extend token"); } + // Mint the JWT with the same expiry the cookie will carry so the token + // and the cookie die together. Without an `expiresAt` claim the + // validator at `utils/jwt.ts:47` skips the expiry check, leaving the + // token replayable indefinitely if it ever leaves the cookie. + const expires = addDays(new Date(), JWT_COOKIE_TTL_DAYS); + const jwtToken = user.getJwtToken(expires, service); + // ensure that the lastActiveAt on user is updated to prevent replay requests await user.updateActiveAt(ctx, true); ctx.cookies.set("accessToken", jwtToken, { sameSite: "lax", - expires: addMonths(new Date(), 3), + expires, }); const [team, collection, view] = await Promise.all([ Team.findByPk(user.teamId), diff --git a/server/utils/authentication.ts b/server/utils/authentication.ts index c6e8a93aa848..538be3b5717d 100644 --- a/server/utils/authentication.ts +++ b/server/utils/authentication.ts @@ -1,5 +1,5 @@ import querystring from "node:querystring"; -import { addMonths } from "date-fns"; +import { addDays } from "date-fns"; import type { Context } from "koa"; import pick from "lodash/pick"; import { Client } from "@shared/types"; @@ -10,6 +10,14 @@ import { Event, Collection, View } from "@server/models"; import type { APIContext, AuthenticationResult } from "@server/types"; import { AuthenticationType } from "@server/types"; +/** + * Lifetime of the JWT cookie (`accessToken`) issued after Cognito sign-in. + * 7 days matches the rest of the foss-server-bundle-devstack + * (Plane / Penpot / SurfSense / Twenty / oauth2-proxy). Single source of + * truth for all three call sites that mint this cookie. + */ +export const JWT_COOKIE_TTL_DAYS = 7; + /** * Parse and return the details from the "sessions" cookie in the request, if * any. The cookie is on the apex domain and includes session details for @@ -88,7 +96,7 @@ export async function signIn( ); const domain = getCookieDomain(ctx.request.hostname, env.isCloudHosted); - const expires = addMonths(new Date(), 3); + const expires = addDays(new Date(), JWT_COOKIE_TTL_DAYS); // set a cookie for which service we last signed in with. This is // only used to display a UI hint for the user for next time