fix(auth): bind accessToken cookie + JWT to a 7-day TTL#13
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes the accessToken cookie expiration to a 7-day TTL and centralizes the TTL value so it can be reused across multiple authentication flows.
Changes:
- Introduces a shared
JWT_COOKIE_TTL_DAYSconstant for theaccessTokencookie lifetime. - Updates
accessTokencookie expiry from 3 months to 7 days in the sign-in flow,/auth/redirect, and ForwardAuth middleware issuance. - Adds middleware test coverage asserting the ForwardAuth-issued cookie expiry aligns with the configured TTL.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| server/utils/authentication.ts | Adds shared TTL constant and applies it to sign-in cookie expiry. |
| server/routes/auth/index.ts | Applies the shared TTL constant to the /auth/redirect cookie expiry. |
| server/middlewares/authentication.ts | Uses shared TTL constant when minting cookies for ForwardAuth. |
| server/middlewares/authentication.test.ts | Adds a test validating the ForwardAuth accessToken cookie expiry timing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * 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. | ||
| */ |
| 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 { 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"; |
|
|
||
| ctx.cookies.set("accessToken", jwtToken, { | ||
| sameSite: "lax", | ||
| expires: addMonths(new Date(), 3), | ||
| expires: addDays(new Date(), JWT_COOKIE_TTL_DAYS), | ||
| }); |
There was a problem hiding this comment.
Good catch on the JWT-no-expiresAt claim — but worth flagging that this is a pre-existing upstream bug, not introduced by this PR. The user.getJwtToken(undefined, service) call on line 36 is unchanged; only the cookie's expires line was modified.
Net effect on exposure window:
- Before this PR: 3-month cookie + eternal JWT → 90-day replay window
- After this PR: 7-day cookie + eternal JWT → 7-day replay window
So this PR actually shrinks the exposure ~13x. It doesn't introduce the mismatch.
Will file a separate follow-up to pass expires into getJwtToken at this call site (matching the pattern already used in middlewares/authentication.ts and utils/authentication.ts). Keeping this PR scoped to cookie TTL only.
de893b8 to
306a0c6
Compare
306a0c6 to
3a95df7
Compare
| // 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"); |
There was a problem hiding this comment.
Already addressed in the current revision — the comparison-based check has been replaced with an explicit type check exactly as suggested:
if (getJWTPayload(ctx.state.auth.token).type !== "transfer") {
throw AuthenticationError("Cannot extend token");
}This decodes the incoming token's payload and requires type === "transfer" for the route to proceed, rather than relying on the JWT-determinism heuristic that the expiresAt fix invalidated. The existing test "should prevent token extension by rejecting JWT tokens" exercises this — a session JWT now reaches the route handler and gets rejected at the type check.
The diff hunk Copilot is reading actually shows the fix already applied (the - line is the old comparison, the + lines are the type check). Looks like a review-rendering issue where the bot is treating the pre-image of the hunk as active.
| // 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], "base64").toString() |
| * 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. | ||
| */ |
| // This route is only for exchanging a short-lived transfer token for a | ||
| // session cookie. Reject anything else (in particular session JWTs being |
| expect(state.auth.type).toEqual(AuthenticationType.APP); | ||
| }); | ||
|
|
||
| it("should issue the accessToken cookie with a 7-day expiry", async () => { |
Outline's accessToken cookie was hardcoded to 3 months
(addMonths(new Date(), 3)) at three call sites — outliving the rest of
the foss-server-bundle's 7-day session window. After a stack-wide
session expiry users would stay signed in to Outline alone.
Changes:
- Export JWT_COOKIE_TTL_DAYS = 7 from server/utils/authentication.ts as
a single source of truth (with rationale in a docstring).
- Use addDays(new Date(), JWT_COOKIE_TTL_DAYS) at all three mint sites:
- server/middlewares/authentication.ts (FORWARDAUTH login)
- server/routes/auth/index.ts (/auth/redirect)
- server/utils/authentication.ts (signIn callback)
- Fix a pre-existing upstream issue at /auth/redirect: getJwtToken was
called with no expiresAt arg, producing a JWT the validator at
utils/jwt.ts:47 never rejects (claim missing → check skipped). Now
passes `expires` into both getJwtToken and the cookie set, matching
the pattern the other two paths already use.
- Tests cover both halves:
- middlewares/authentication.test.ts asserts the FORWARDAUTH cookie's
expires is ~now + JWT_COOKIE_TTL_DAYS (±60s).
- routes/auth/index.test.ts asserts /auth/redirect mints a JWT whose
expiresAt claim is set and ~now + JWT_COOKIE_TTL_DAYS (±60s).
In future, lift JWT_COOKIE_TTL_DAYS to an env var if deployment-specific
control is needed.
3a95df7 to
ad431a1
Compare
Summary
Outline's
accessTokencookie was hardcoded to 3 months (addMonths(new Date(), 3)) at three call sites — outliving the rest of thefoss-server-bundle's 7-day session window. After a stack-wide session expiry users would stay signed in to Outline alone.Additionally, the
/auth/redirecthandler was minting JWTs with noexpiresAtclaim (getJwtToken(undefined, service)), making the token replayable indefinitely if it ever left the cookie. The validator atserver/utils/jwt.ts:47silently skips the expiry check when the claim is missing.Changes
JWT_COOKIE_TTL_DAYS = 7fromserver/utils/authentication.tsas a single source of truth (with rationale in a docstring).addDays(new Date(), JWT_COOKIE_TTL_DAYS)at all three mint sites:server/middlewares/authentication.ts(FORWARDAUTH login)server/routes/auth/index.ts(/auth/redirect)server/utils/authentication.ts(signIncallback)/auth/redirectto passexpiresinto bothgetJwtTokenand the cookie option, so the JWT and cookie die together. Matches the pattern the other two paths already use.jwtToken === ctx.state.auth.tokenheuristic at/auth/redirect(which relied on session JWTs being deterministic — no longer true after theexpiresAtfix) with an explicit type check:getJWTPayload(ctx.state.auth.token).type !== "transfer". Direct, intent-revealing.Tests
server/middlewares/authentication.test.ts— asserts the FORWARDAUTHaccessTokencookie'sexpiresis~now + JWT_COOKIE_TTL_DAYS(±60s skew).server/routes/auth/index.test.ts— asserts/auth/redirectmints a JWT whoseexpiresAtclaim is set and~now + JWT_COOKIE_TTL_DAYS(±60s skew). Guards against regression togetJwtToken(undefined, ...). JWT payload is decoded viabase64urlper spec.should prevent token extension by rejecting JWT tokenstest still passes via the new type check.Future
If deployment-specific control becomes needed, lift
JWT_COOKIE_TTL_DAYSto an env var (e.g.SESSION_TTL_SECONDS).