Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 0 additions & 42 deletions server/middlewares/authentication.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}) {
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions server/middlewares/authentication.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
37 changes: 0 additions & 37 deletions server/routes/auth/index.test.ts
Original file line number Diff line number Diff line change
@@ -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();

Expand Down Expand Up @@ -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);
});
});
23 changes: 5 additions & 18 deletions server/routes/auth/index.ts
Original file line number Diff line number Diff line change
@@ -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<AppState, AppContext>();
const router = new Router();
Expand All @@ -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),
Expand Down
12 changes: 2 additions & 10 deletions server/utils/authentication.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading