From 174e774dbbd95e2f8b7ad6ea4ffacb86a99499a9 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Wed, 13 May 2026 18:22:04 +0500 Subject: [PATCH] Revert "fix(auth): bind accessToken cookie + JWT to a 7-day TTL (#13)" This reverts commit 46324736c6e31cf06098be1bbd126ee90aa56535. --- server/middlewares/authentication.test.ts | 42 ----------------------- server/middlewares/authentication.ts | 5 ++- server/routes/auth/index.test.ts | 37 -------------------- server/routes/auth/index.ts | 23 +++---------- server/utils/authentication.ts | 12 ++----- 5 files changed, 9 insertions(+), 110 deletions(-) diff --git a/server/middlewares/authentication.test.ts b/server/middlewares/authentication.test.ts index 6ffd0482da8c..0ecb2a37f19d 100644 --- a/server/middlewares/authentication.test.ts +++ b/server/middlewares/authentication.test.ts @@ -11,7 +11,6 @@ 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 = {}) { @@ -380,47 +379,6 @@ 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 83bd7ec2787d..2160e6008d09 100644 --- a/server/middlewares/authentication.ts +++ b/server/middlewares/authentication.ts @@ -1,4 +1,4 @@ -import { addDays } from "date-fns"; +import { addMonths } from "date-fns"; import type { Next } from "koa"; import capitalize from "lodash/capitalize"; import { Op } from "sequelize"; @@ -18,7 +18,6 @@ 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, @@ -57,7 +56,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 = addDays(new Date(), JWT_COOKIE_TTL_DAYS); + const expires = addMonths(new Date(), 3); 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 9f29fc5a97d1..61bd5f308b04 100644 --- a/server/routes/auth/index.test.ts +++ b/server/routes/auth/index.test.ts @@ -1,6 +1,5 @@ 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(); @@ -44,40 +43,4 @@ 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 d0d6be6c52b1..ccec003e5fbd 100644 --- a/server/routes/auth/index.ts +++ b/server/routes/auth/index.ts @@ -1,17 +1,15 @@ import passport from "@outlinewiki/koa-passport"; -import { addDays } from "date-fns"; +import { addMonths } 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 { JWT_COOKIE_TTL_DAYS } from "@server/utils/authentication"; -import { getJWTPayload } from "@server/utils/jwt"; +import { verifyCSRFToken } from "@server/middlewares/csrf"; const app = new Koa(); const router = new Router(); @@ -34,29 +32,18 @@ void (async () => { router.get("/redirect", authMiddleware(), async (ctx: APIContext) => { const { user, service } = ctx.state.auth; + const jwtToken = user.getJwtToken(undefined, service); - // 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") { + if (jwtToken === ctx.state.auth.token) { 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, + expires: addMonths(new Date(), 3), }); const [team, collection, view] = await Promise.all([ Team.findByPk(user.teamId), diff --git a/server/utils/authentication.ts b/server/utils/authentication.ts index 538be3b5717d..c6e8a93aa848 100644 --- a/server/utils/authentication.ts +++ b/server/utils/authentication.ts @@ -1,5 +1,5 @@ import querystring from "node:querystring"; -import { addDays } from "date-fns"; +import { addMonths } from "date-fns"; import type { Context } from "koa"; import pick from "lodash/pick"; import { Client } from "@shared/types"; @@ -10,14 +10,6 @@ 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 @@ -96,7 +88,7 @@ export async function signIn( ); const domain = getCookieDomain(ctx.request.hostname, env.isCloudHosted); - const expires = addDays(new Date(), JWT_COOKIE_TTL_DAYS); + const expires = addMonths(new Date(), 3); // 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