sign the frame cookie instead of storing it
The minted tokens lived in an in-memory Map, so every restart of the platform
silently invalidated every open frame: the browser kept sending a cookie that
matched nothing and the frame answered 401 mid-session with "no gitea frame
session". Deploying anything at all logged everyone out of the iframe — which
is exactly how this was found, a restart landing between a sign-in and the POST
that followed it.
An HMAC over the account and an expiry needs no state, so a restart changes
nothing. Same shape api/browser/relay-auth.ts already uses, and the same key —
getKey('jwt') from the secret store, which persists.
Compared with timingSafeEqual, because comparing a signature with === leaks how
much of it was right.
This commit is contained in:
+30
-21
@@ -1,4 +1,5 @@
|
||||
import { randomBytes } from 'node:crypto';
|
||||
import { createHmac, timingSafeEqual } from 'node:crypto';
|
||||
import { getKey } from 'officerdb/secret-store';
|
||||
import { createRouter } from '@@/create-router';
|
||||
import { createSidecarProxy } from '@@/sidecar/create-proxy';
|
||||
import { mountPrefix } from '@@/plugins/manifest';
|
||||
@@ -127,27 +128,29 @@ export const router = router_;
|
||||
export const getGiteaServerUrl = proxy.getHttpUrl;
|
||||
|
||||
// ── The cookie ──
|
||||
//
|
||||
// SIGNED, not stored. The first version held minted tokens in a `Map`, which meant every restart of the
|
||||
// platform silently invalidated every open frame — the browser kept sending a cookie that matched
|
||||
// nothing, and the frame answered 401 mid-session with "no gitea frame session". Deploying anything at
|
||||
// all logged everyone out of the iframe.
|
||||
//
|
||||
// An HMAC over the account and an expiry needs no state, so a restart changes nothing. Same shape
|
||||
// `api/browser/relay-auth.ts` already uses, and the same key: `getKey('jwt')` from the secret store,
|
||||
// which persists.
|
||||
|
||||
const COOKIE = 'officer_gitea_frame';
|
||||
const TTL_MS = 12 * 60 * 60 * 1000;
|
||||
|
||||
/** token → { userId, expires }. In memory on purpose: a restart closing every frame is correct. */
|
||||
const sessions = new Map<string, { userId: number; expires: number }>();
|
||||
const sign = (payload: string): string => createHmac('sha256', getKey('jwt')).update(`gitea-frame:${payload}`).digest('base64url');
|
||||
|
||||
const sweep = () => {
|
||||
const now = Date.now();
|
||||
for (const [token, s] of sessions) if (s.expires < now) sessions.delete(token);
|
||||
};
|
||||
|
||||
/** Mint a frame token for an authenticated caller, and the Set-Cookie that carries it. */
|
||||
/** Mint the Set-Cookie an authenticated caller needs before the frame loads. */
|
||||
export function mintFrameCookie(userId: number, prefix: string): string {
|
||||
sweep();
|
||||
const token = randomBytes(32).toString('base64url');
|
||||
sessions.set(token, { userId, expires: Date.now() + TTL_MS });
|
||||
const payload = `${userId}.${Date.now() + TTL_MS}`;
|
||||
const value = `${payload}.${sign(payload)}`;
|
||||
// Path-scoped to the frame, so it is never sent to the rest of the platform. `Lax` rather than `None`
|
||||
// because the frame is same-origin — which is the entire reason this is proxied instead of pointed at
|
||||
// gitea.officer.dev, where the cookie would be third-party and dropped.
|
||||
return `${COOKIE}=${token}; Path=${prefix}/_web; Max-Age=${Math.floor(TTL_MS / 1000)}; HttpOnly; SameSite=Lax`;
|
||||
// the Gitea host, where the cookie would be third-party and dropped.
|
||||
return `${COOKIE}=${value}; Path=${prefix}/_web; Max-Age=${Math.floor(TTL_MS / 1000)}; HttpOnly; SameSite=Lax`;
|
||||
}
|
||||
|
||||
/** The user this request's frame cookie belongs to, or null. */
|
||||
@@ -158,11 +161,17 @@ export function frameUser(cookieHeader: string | null): number | null {
|
||||
.map((p) => p.trim())
|
||||
.find((p) => p.startsWith(`${COOKIE}=`));
|
||||
if (!raw) return null;
|
||||
const found = sessions.get(raw.slice(COOKIE.length + 1));
|
||||
if (!found) return null;
|
||||
if (found.expires < Date.now()) {
|
||||
sessions.delete(raw.slice(COOKIE.length + 1));
|
||||
return null;
|
||||
}
|
||||
return found.userId;
|
||||
|
||||
const [id, expires, mac] = raw.slice(COOKIE.length + 1).split('.');
|
||||
if (!id || !expires || !mac) return null;
|
||||
|
||||
const expected = sign(`${id}.${expires}`);
|
||||
// Constant-time, because comparing a signature with `===` leaks how much of it was right.
|
||||
const a = Buffer.from(mac);
|
||||
const b = Buffer.from(expected);
|
||||
if (a.length !== b.length || !timingSafeEqual(a, b)) return null;
|
||||
if (Number(expires) < Date.now()) return null;
|
||||
|
||||
const userId = Number(id);
|
||||
return Number.isInteger(userId) && userId > 0 ? userId : null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user