From bda129c0a4e7bb40ec9e9556136c4115f29df689 Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Tue, 16 Jun 2026 17:27:11 +0200 Subject: [PATCH 1/4] fix: fix api reauth --- client/utils/apiFetch.ts | 75 ++++++++++++++++++++++++++++--- server/kf/api.ts | 13 ++++++ server/middleware/silentReauth.ts | 28 +++++++----- server/server.ts | 4 ++ 4 files changed, 102 insertions(+), 18 deletions(-) diff --git a/client/utils/apiFetch.ts b/client/utils/apiFetch.ts index d3de7f8e95..c0eba68e14 100644 --- a/client/utils/apiFetch.ts +++ b/client/utils/apiFetch.ts @@ -23,7 +23,50 @@ type HttpMethod = (typeof httpMethods)[number]; type ApiFetch = ApiFetchFn & { [K in HttpMethod]: HttpMethodApiFetchWrapper }; -export const apiFetch = ((path, opts) => { +// ── Silent session renewal via hidden iframe ── + +let renewalInFlight: Promise | null = null; + +function renewSession(): Promise { + if (renewalInFlight) return renewalInFlight; + + renewalInFlight = new Promise((resolve) => { + const iframe = document.createElement('iframe'); + iframe.style.display = 'none'; + + const cleanup = () => { + window.removeEventListener('message', onMessage); + clearTimeout(timeout); + iframe.remove(); + renewalInFlight = null; + }; + + const onMessage = (event: MessageEvent) => { + if ( + event.origin === window.location.origin && + event.data?.type === 'pubpub:session-renewed' + ) { + cleanup(); + resolve(!!event.data.success); + } + }; + + const timeout = setTimeout(() => { + cleanup(); + resolve(false); + }, 15_000); + + window.addEventListener('message', onMessage); + iframe.src = '/auth/login?renew=true&return_to=/auth/renew-done'; + document.body.appendChild(iframe); + }); + + return renewalInFlight; +} + +// ── Core fetch wrapper ── + +function rawFetch(path: string, opts?: RequestInit): Promise { return fetch(path, { ...opts, headers: { @@ -31,14 +74,32 @@ export const apiFetch = ((path, opts) => { 'Content-Type': 'application/json', }, credentials: 'include', - }).then((response) => { + }); +} + +export const apiFetch = ((path, opts) => { + return rawFetch(path, opts).then(async (response) => { if (!response.ok) { - return response.json().then((err) => { - if (response.status === 423 && err?.error === 'readOnly') { - window.dispatchEvent(new CustomEvent('pubpub:readOnly')); + const err = await response.json(); + + // Session expired but was previously logged in — try silent renewal + if (response.status === 401 && err?.error === 'sessionExpired') { + const renewed = await renewSession(); + if (renewed) { + const retry = await rawFetch(path, opts); + if (retry.ok) return retry.json(); + throw await retry.json(); } - throw err; - }); + // Renewal failed — full page reload so the page-level reauth kicks in + window.location.reload(); + // Never resolves — the reload navigates away + return new Promise(() => {}); + } + + if (response.status === 423 && err?.error === 'readOnly') { + window.dispatchEvent(new CustomEvent('pubpub:readOnly')); + } + throw err; } return response.json(); }); diff --git a/server/kf/api.ts b/server/kf/api.ts index 0428807a4e..d3c556627a 100644 --- a/server/kf/api.ts +++ b/server/kf/api.ts @@ -395,6 +395,19 @@ router.get('/auth/renew-failed', (req: any, res: any) => { return res.redirect(returnTo); }); +// ─── Iframe renew completion (postMessage to parent) ──────────────── +// The frontend opens a hidden iframe to /auth/login?renew=true&return_to=/auth/renew-done +// After the OIDC prompt=none dance, the iframe lands here and signals the parent. +router.get('/auth/renew-done', (req: any, res: any) => { + const success = !!req.user; + res.set('Cache-Control', 'no-store'); + return res.send( + ``, + ); +}); + // ─── Logout ────────────────────────────────────────────────────────── router.post('/auth/logout', (req: any, res: any) => { diff --git a/server/middleware/silentReauth.ts b/server/middleware/silentReauth.ts index 0767980e77..0cf0564152 100644 --- a/server/middleware/silentReauth.ts +++ b/server/middleware/silentReauth.ts @@ -1,11 +1,13 @@ import type { NextFunction, Request, Response } from 'express'; -const SKIP_PREFIXES = ['/api', '/auth', '/dist', '/static', '/service-worker', '/favicon']; +const PAGE_SKIP_PREFIXES = ['/auth', '/dist', '/static', '/service-worker', '/favicon']; /** - * Detects "was logged in, session expired" and triggers silent re-auth - * via OIDC prompt=none. Only fires for browser page loads (GET requests - * to non-API, non-asset paths). + * Detects "was logged in, session expired" and triggers silent re-auth. + * + * - Page navigations (GET to non-asset paths): 302 redirect to OIDC prompt=none. + * - API requests (/api/*): 401 JSON with `error: 'sessionExpired'` so the + * frontend can renew via a hidden iframe and retry transparently. * * Uses the `pp-lic` CDN cookie (set at login, 30-day maxAge) to detect * that the user was previously authenticated. A `pp-renew-failed` cookie @@ -14,20 +16,24 @@ const SKIP_PREFIXES = ['/api', '/auth', '/dist', '/static', '/service-worker', ' */ export const silentReauthMiddleware = () => { return (req: Request, res: Response, next: NextFunction) => { - if (req.method !== 'GET') return next(); - if (SKIP_PREFIXES.some((p) => req.path.startsWith(p))) { - return next(); - } - if (req.user) return next(); - // After logout it's set to 'pp-lo' - renewing would resurrect the session the user just deliberately ended. + // After logout it's set to 'pp-lo' — renewing would resurrect the session the user just deliberately ended. const lic = req.cookies?.['pp-lic']; if (typeof lic !== 'string' || !lic.startsWith('pp-li-')) return next(); - // Circuit breaker: recently tried and failed - skip + // Circuit breaker: recently tried and failed — skip if (req.cookies?.['pp-renew-failed']) return next(); + // ── API requests: tell the frontend to renew ── + if (req.path.startsWith('/api')) { + return res.status(401).json({ error: 'sessionExpired' }); + } + + // ── Page navigations: redirect through OIDC prompt=none ── + if (req.method !== 'GET') return next(); + if (PAGE_SKIP_PREFIXES.some((p) => req.path.startsWith(p))) return next(); + // This 302 carries no Set-Cookie, so Fastly would otherwise cache it // under the per-`pp-lic` cache key (vcl_hash only mixes connect.sid in // for /api routes). A cached "go reauth" redirect would then be served diff --git a/server/server.ts b/server/server.ts index abbdb30eac..a04ae7a350 100755 --- a/server/server.ts +++ b/server/server.ts @@ -138,6 +138,10 @@ appRouter.use( secret: env.SESSION_SECRET ?? 'sessionsecret', resave: false, saveUninitialized: false, + // Reset cookie maxAge on every response so the session expires after + // N minutes of *inactivity* rather than N minutes since login. The + // store's touch() keeps the DB row in sync without a full resave. + rolling: true, // TLS is terminated at the edge (Fastly) and forwarded as plain HTTP, // so without trusting the proxy express-session sees an insecure // connection and silently drops the `secure` cookie. This honors From 494aa5d48daeabeccbff6a5b7500c17ebb801f37 Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Tue, 16 Jun 2026 21:11:53 +0200 Subject: [PATCH 2/4] fix: also make work for altcha --- client/components/Altcha/Altcha.tsx | 12 +++++++- client/utils/apiFetch.ts | 48 ++++++++++++++++++++--------- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/client/components/Altcha/Altcha.tsx b/client/components/Altcha/Altcha.tsx index 1289a6c381..d3fe177371 100644 --- a/client/components/Altcha/Altcha.tsx +++ b/client/components/Altcha/Altcha.tsx @@ -1,5 +1,6 @@ import React, { forwardRef, useEffect, useImperativeHandle, useRef, useState } from 'react'; +import { apiFetchRaw } from 'client/utils/apiFetch'; import { usePageContext } from 'utils/hooks'; export type AltchaRef = { @@ -40,9 +41,17 @@ const Altcha = forwardRef((props, ref) => { if (!loaded) return; const w = widgetRef.current; if (!w) return; + // Route the widget's challenge fetch through apiFetchRaw so it carries + // credentials and, crucially, survives an expired session: a logged-in + // user whose short session has lapsed gets `401 sessionExpired` from + // silentReauthMiddleware on /api/* — the widget's own plain fetch can't + // handle that and the captcha (and thus pub creation) silently fails. + // apiFetchRaw renews via the hidden iframe and retries. + + // needs to be done like this, doesnt work when just passing it to altcha-widget + (w as unknown as { customfetch?: typeof apiFetchRaw }).customfetch = apiFetchRaw; const handleStateChange = (ev: Event) => { const e = ev as CustomEvent<{ payload?: string; state: string }>; - console.log('state changed', e.detail); switch (e.detail.state) { case 'error': @@ -126,6 +135,7 @@ const Altcha = forwardRef((props, ref) => { delay={500} ref={widgetRef as any} challengeurl={challengeurl} + customfetch={apiFetchRaw} {...(auto ? { auto } : {})} floating="auto" {...devAttrs} diff --git a/client/utils/apiFetch.ts b/client/utils/apiFetch.ts index c0eba68e14..138c68ebba 100644 --- a/client/utils/apiFetch.ts +++ b/client/utils/apiFetch.ts @@ -77,25 +77,43 @@ function rawFetch(path: string, opts?: RequestInit): Promise { }); } +/** + * Like fetch, but transparently handles an expired-but-renewable session: + * on a `401 {error: 'sessionExpired'}` it silently renews (hidden iframe → + * OIDC prompt=none) and retries once, returning the resolved Response. Returns + * credentials with every request. Use this for callers that need a raw + * Response and the renewal behaviour — e.g. the Altcha widget's customfetch, + * which otherwise issues a plain fetch that dies on the 401. + */ +export async function apiFetchRaw(path: string, opts?: RequestInit): Promise { + const response = await rawFetch(path, opts); + if (response.status === 401) { + // Peek the body without consuming it for the caller. + const err = await response + .clone() + .json() + .catch(() => null); + if (err?.error === 'sessionExpired') { + const renewed = await renewSession(); + if (renewed) { + return rawFetch(path, opts); + } + // Renewal failed — full page reload so the page-level reauth kicks in + window.location.reload(); + // Never resolves — the reload navigates away + return new Promise(() => { + /* page is reloading */ + }); + } + } + return response; +} + export const apiFetch = ((path, opts) => { - return rawFetch(path, opts).then(async (response) => { + return apiFetchRaw(path, opts).then(async (response) => { if (!response.ok) { const err = await response.json(); - // Session expired but was previously logged in — try silent renewal - if (response.status === 401 && err?.error === 'sessionExpired') { - const renewed = await renewSession(); - if (renewed) { - const retry = await rawFetch(path, opts); - if (retry.ok) return retry.json(); - throw await retry.json(); - } - // Renewal failed — full page reload so the page-level reauth kicks in - window.location.reload(); - // Never resolves — the reload navigates away - return new Promise(() => {}); - } - if (response.status === 423 && err?.error === 'readOnly') { window.dispatchEvent(new CustomEvent('pubpub:readOnly')); } From 6ac0d2a2b9b3133e46789023d71c969cdc28db20 Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Wed, 17 Jun 2026 15:22:33 +0200 Subject: [PATCH 3/4] fix: don't drop headers --- client/utils/apiFetch.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/client/utils/apiFetch.ts b/client/utils/apiFetch.ts index 138c68ebba..cef7e54e6d 100644 --- a/client/utils/apiFetch.ts +++ b/client/utils/apiFetch.ts @@ -72,6 +72,7 @@ function rawFetch(path: string, opts?: RequestInit): Promise { headers: { Accept: 'application/json', 'Content-Type': 'application/json', + ...opts?.headers, }, credentials: 'include', }); From 76612261c8864f17ad5b4d92a667c5df9aa268b9 Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Thu, 18 Jun 2026 15:57:46 +0200 Subject: [PATCH 4/4] refactor: rely on refresh tokens instead, like we should have done from the beginning --- client/components/Altcha/Altcha.tsx | 7 +- client/utils/apiFetch.ts | 81 +---------- server/kf/api.ts | 213 ++++------------------------ server/kf/oidc.server.ts | 77 +++++++++- server/kf/sessionCheck.ts | 130 +++++++++++++++++ server/middleware/kfSessionCheck.ts | 30 ++++ server/middleware/silentReauth.ts | 49 ------- server/server.ts | 13 +- types/global.d.ts | 10 +- 9 files changed, 284 insertions(+), 326 deletions(-) create mode 100644 server/kf/sessionCheck.ts create mode 100644 server/middleware/kfSessionCheck.ts delete mode 100644 server/middleware/silentReauth.ts diff --git a/client/components/Altcha/Altcha.tsx b/client/components/Altcha/Altcha.tsx index d3fe177371..8ba5509c0f 100644 --- a/client/components/Altcha/Altcha.tsx +++ b/client/components/Altcha/Altcha.tsx @@ -42,12 +42,7 @@ const Altcha = forwardRef((props, ref) => { const w = widgetRef.current; if (!w) return; // Route the widget's challenge fetch through apiFetchRaw so it carries - // credentials and, crucially, survives an expired session: a logged-in - // user whose short session has lapsed gets `401 sessionExpired` from - // silentReauthMiddleware on /api/* — the widget's own plain fetch can't - // handle that and the captcha (and thus pub creation) silently fails. - // apiFetchRaw renews via the hidden iframe and retries. - + // credentials (the widget's own plain fetch would not). // needs to be done like this, doesnt work when just passing it to altcha-widget (w as unknown as { customfetch?: typeof apiFetchRaw }).customfetch = apiFetchRaw; const handleStateChange = (ev: Event) => { diff --git a/client/utils/apiFetch.ts b/client/utils/apiFetch.ts index cef7e54e6d..6348061ae4 100644 --- a/client/utils/apiFetch.ts +++ b/client/utils/apiFetch.ts @@ -23,50 +23,15 @@ type HttpMethod = (typeof httpMethods)[number]; type ApiFetch = ApiFetchFn & { [K in HttpMethod]: HttpMethodApiFetchWrapper }; -// ── Silent session renewal via hidden iframe ── - -let renewalInFlight: Promise | null = null; - -function renewSession(): Promise { - if (renewalInFlight) return renewalInFlight; - - renewalInFlight = new Promise((resolve) => { - const iframe = document.createElement('iframe'); - iframe.style.display = 'none'; - - const cleanup = () => { - window.removeEventListener('message', onMessage); - clearTimeout(timeout); - iframe.remove(); - renewalInFlight = null; - }; - - const onMessage = (event: MessageEvent) => { - if ( - event.origin === window.location.origin && - event.data?.type === 'pubpub:session-renewed' - ) { - cleanup(); - resolve(!!event.data.success); - } - }; - - const timeout = setTimeout(() => { - cleanup(); - resolve(false); - }, 15_000); - - window.addEventListener('message', onMessage); - iframe.src = '/auth/login?renew=true&return_to=/auth/renew-done'; - document.body.appendChild(iframe); - }); - - return renewalInFlight; -} - // ── Core fetch wrapper ── -function rawFetch(path: string, opts?: RequestInit): Promise { +/** + * Like fetch, but always sends credentials and JSON headers and returns the raw + * Response. Use this for callers that need the Response object rather than the + * parsed JSON — e.g. the Altcha widget's `customfetch`, which needs the + * credentialed request but its own response handling. + */ +export function apiFetchRaw(path: string, opts?: RequestInit): Promise { return fetch(path, { ...opts, headers: { @@ -78,38 +43,6 @@ function rawFetch(path: string, opts?: RequestInit): Promise { }); } -/** - * Like fetch, but transparently handles an expired-but-renewable session: - * on a `401 {error: 'sessionExpired'}` it silently renews (hidden iframe → - * OIDC prompt=none) and retries once, returning the resolved Response. Returns - * credentials with every request. Use this for callers that need a raw - * Response and the renewal behaviour — e.g. the Altcha widget's customfetch, - * which otherwise issues a plain fetch that dies on the 401. - */ -export async function apiFetchRaw(path: string, opts?: RequestInit): Promise { - const response = await rawFetch(path, opts); - if (response.status === 401) { - // Peek the body without consuming it for the caller. - const err = await response - .clone() - .json() - .catch(() => null); - if (err?.error === 'sessionExpired') { - const renewed = await renewSession(); - if (renewed) { - return rawFetch(path, opts); - } - // Renewal failed — full page reload so the page-level reauth kicks in - window.location.reload(); - // Never resolves — the reload navigates away - return new Promise(() => { - /* page is reloading */ - }); - } - } - return response; -} - export const apiFetch = ((path, opts) => { return apiFetchRaw(path, opts).then(async (response) => { if (!response.ok) { diff --git a/server/kf/api.ts b/server/kf/api.ts index d3c556627a..539db97aea 100644 --- a/server/kf/api.ts +++ b/server/kf/api.ts @@ -24,6 +24,7 @@ import { promisify } from 'util'; import { Collection, Community, Member, Pub, PubAttribution, Release, User } from 'server/models'; import { sequelize } from 'server/sequelize'; +import { logout } from 'server/utils/logout'; import { getHashedUserId } from 'utils/caching/getHashedUserId'; import { ensureUserIsCommunityAdmin } from 'utils/ensureUserIsCommunityAdmin'; import { isDevelopment, isDuqDuq, isProd } from 'utils/environment'; @@ -40,6 +41,7 @@ import { OIDC_ISSUER_URL, } from './oidc.server'; import { provisionLocalUser } from './provisionLocalUser'; +import { setKfSessionTokens } from './sessionCheck'; import { handleSessionRevoked, handleUserBanned, @@ -93,60 +95,6 @@ function isPlatformSubdomain(host: string): boolean { return host.endsWith('.pubpub.org') || host.endsWith('.duqduq.org'); } -/** - * Set the silent-reauth circuit breaker. It MUST be visible on the community - * origin where silentReauthMiddleware checks it. On platform subdomains we - * scope it to the parent domain (like pp-lic) so a breaker set on www.* is - * also sent to the community subdomain; on custom domains it's host-only - * (and must be set on that origin — see failRenew / /auth/renew-failed). - */ -function setRenewFailedCookie(req: any, res: any): void { - const onPlatformSubdomain = (isProd() || isDuqDuq()) && isPlatformSubdomain(req.hostname); - res.cookie('pp-renew-failed', '1', { - maxAge: 60 * 60 * 1000, - httpOnly: true, - ...(onPlatformSubdomain && { domain: isDuqDuq() ? '.duqduq.org' : '.pubpub.org' }), - }); -} - -/** Clear the breaker — matching whatever domain scope setRenewFailedCookie used. */ -function clearRenewFailedCookie(req: any, res: any): void { - res.clearCookie('pp-renew-failed'); - if ((isProd() || isDuqDuq()) && isPlatformSubdomain(req.hostname)) { - res.clearCookie('pp-renew-failed', { domain: isDuqDuq() ? '.duqduq.org' : '.pubpub.org' }); - } -} - -/** - * Bail out of a silent renewal without a session: trip the circuit breaker so - * silentReauthMiddleware stops redirecting, then send the user back to where - * they came from (anonymous). Used on every renewal failure path so a single - * failure degrades to "anonymous page for 1h" instead of an infinite loop. - * Crucially never returns a 500 — Fastly restarts 500s on GET, which would - * re-redeem the one-time OIDC code (→ invalid_grant). - */ -function failRenew(req: any, res: any, host: string | undefined, returnTo: string): void { - const protocol = isDevelopment() ? 'http' : 'https'; - // Custom domains: this callback runs on the platform domain (www.*), which - // can't set a cookie for the custom origin. Bounce through /auth/renew-failed - // on that origin so the breaker lands where silentReauthMiddleware checks it. - if (host && !isPlatformSubdomain(host) && host !== req.hostname) { - res.redirect( - `${protocol}://${host}/auth/renew-failed?return_to=${encodeURIComponent(returnTo)}`, - ); - return; - } - // Platform subdomains (and same-origin): a parent-domain-scoped breaker set - // here is visible across *.duqduq.org / *.pubpub.org, including the community - // subdomain the user is actually on. - setRenewFailedCookie(req, res); - if (host && host !== req.hostname) { - res.redirect(`${protocol}://${host}${returnTo}`); - return; - } - res.redirect(returnTo); -} - // ── Router ─────────────────────────────────────────────────────────── export const router = Router(); @@ -161,32 +109,14 @@ router.get('/auth/login', async (req: any, res: any) => { ? rawReturn : '/'; - const isRenew = req.query.renew === 'true'; - - // Pin silent renewals to the account the expired session belonged to. - // The pp-lic cookie carries the hashed user id of the last login on - // this domain; the callback compares it against the renewed identity. - const currentLic = req.cookies?.['pp-lic']; - const expectedLic = - isRenew && typeof currentLic === 'string' && currentLic.startsWith('pp-li-') - ? currentLic - : undefined; - const codeVerifier = generateCodeVerifier(); const stateToken = encryptPayload({ v: codeVerifier, h: communityHost, r: returnTo, - ...(isRenew && { renew: true }), - ...(expectedLic && { e: expectedLic }), }); - const { url } = await buildAuthorizeUrl( - stateToken, - codeVerifier, - communityHost, - isRenew ? 'none' : undefined, - ); + const { url } = await buildAuthorizeUrl(stateToken, codeVerifier, communityHost); return res.redirect(url); }); @@ -198,27 +128,6 @@ router.get('/auth/callback', async (req: any, res: any) => { const { code, state, error } = req.query; if (error) { - // All prompt=none error types mean "silent re-auth can't complete" - const isPromptNoneError = - error === 'login_required' || - error === 'interaction_required' || - error === 'account_selection_required' || - error === 'consent_required'; - if (isPromptNoneError && state) { - const renewState = decryptPayload<{ - v: string; - h: string; - r: string; - renew?: boolean; - }>(state); - if (renewState?.renew) { - // Mirror the success path: in dev the hostname middleware - // rewrites localhost → demo.pubpub.org, so an absolute - // redirect would send the user to production. Relative - // redirects keep the browser on its current origin. - return failRenew(req, res, renewState.h || req.hostname, renewState.r || '/'); - } - } console.error('KF Auth error:', error, req.query.error_description); return res.status(400).send('Authentication failed. Please try again.'); } @@ -231,8 +140,6 @@ router.get('/auth/callback', async (req: any, res: any) => { v: string; h: string; r: string; - renew?: boolean; - e?: string; }>(state); if (!stateData || !stateData.v) { return res.status(400).send('Invalid or expired authentication state.'); @@ -263,19 +170,6 @@ router.get('/auth/callback', async (req: any, res: any) => { const protocol = isDevelopment() ? 'http' : 'https'; - // Silent renewals must come back as the SAME account the expired - // session belonged to. With kf-auth multi-session, the active account - // there may have changed since — switching identities must be a - // deliberate user choice (via interactive login + account picker), - // never a side effect of background renewal. On mismatch, bail out - // and leave the user logged out. - if (stateData.renew && stateData.e) { - const renewedLic = `pp-li-${getHashedUserId(user)}`; - if (renewedLic !== stateData.e) { - return failRenew(req, res, host, returnTo); - } - } - // For custom domains, we can't set a session here (different domain). // Create a one-time encrypted token and redirect to session-set on the origin. if (host && !isPlatformSubdomain(host)) { @@ -283,6 +177,10 @@ router.get('/auth/callback', async (req: any, res: any) => { u: user.id, r: returnTo, s: kfSessionId, + // Hand the refresh token to the custom-domain origin so it can + // store it on the session it creates (server-side revalidation + // needs it there, not here on the platform domain). + rt: tokens.refresh_token, exp: Date.now() + 60_000, // 60 seconds }); const sessionSetUrl = `${protocol}://${host}/auth/session-set?token=${encodeURIComponent(sessionToken)}`; @@ -293,12 +191,11 @@ router.get('/auth/callback', async (req: any, res: any) => { const logIn = promisify(req.logIn.bind(req)); await logIn(user); - if (kfSessionId) { - req.session.kfSessionId = kfSessionId; - } - - // Clear silent re-auth circuit breaker on successful login - clearRenewFailedCookie(req, res); + // Store the refresh token + kf session id and schedule revalidation. + setKfSessionTokens(req, { + refreshToken: tokens.refresh_token, + kfSessionId, + }); const hashedUserId = getHashedUserId(user); res.cookie('pp-lic', `pp-li-${hashedUserId}`, { @@ -313,16 +210,6 @@ router.get('/auth/callback', async (req: any, res: any) => { return res.redirect(returnTo); } catch (err: any) { console.error('OIDC callback error:', err); - // If this was a silent renewal, never surface a 500: Fastly restarts - // 500s on GET (vcl_fetch), which re-redeems the one-time code and - // produces a confusing `invalid_grant`. Trip the breaker and bounce - // the user back anonymously instead — they retry in an hour. - const renewState = decryptPayload<{ h: string; r: string; renew?: boolean }>( - req.query.state, - ); - if (renewState?.renew) { - return failRenew(req, res, renewState.h || req.hostname, renewState.r || '/'); - } const detail = isDuqDuq() ? ` (${err?.message || err})` : ''; return res.status(500).send(`Login failed. Please try again.${detail}`); } @@ -337,9 +224,13 @@ router.get('/auth/session-set', async (req: any, res: any) => { return res.status(400).send('Missing session token.'); } - const data = decryptPayload<{ u: string; r: string; s?: string | null; exp: number }>( - token, - ); + const data = decryptPayload<{ + u: string; + r: string; + s?: string | null; + rt?: string | null; + exp: number; + }>(token); if (!data || !data.u) { return res.status(400).send('Invalid session token.'); } @@ -357,11 +248,9 @@ router.get('/auth/session-set', async (req: any, res: any) => { const logIn = promisify(req.logIn.bind(req)); await logIn(user); - if (data.s) { - req.session.kfSessionId = data.s; - } - - clearRenewFailedCookie(req, res); + // Store the refresh token + kf session id and schedule revalidation + // (this origin is where server-side revalidation will run). + setKfSessionTokens(req, { refreshToken: data.rt, kfSessionId: data.s }); // Set the CDN cache cookie on this domain const hashedUserId = getHashedUserId(user); @@ -373,65 +262,19 @@ router.get('/auth/session-set', async (req: any, res: any) => { return res.redirect(returnTo); } catch (err) { console.error('Session-set error:', err); - // Trip the breaker so a failed transfer falls back to the anonymous - // page instead of looping (and avoid a 500, which Fastly would retry). - setRenewFailedCookie(req, res); return res.status(500).send('Failed to establish session. Please try again.'); } }); -// ─── Silent re-auth circuit breaker for cross-origin communities ───── -// When a renewal fails, the breaker must be set on the community origin (where -// silentReauthMiddleware checks it). For custom domains the OIDC callback runs -// on a different origin and can't set that cookie, so it bounces the browser -// here on the community origin to set it, then returns the user to their page. -router.get('/auth/renew-failed', (req: any, res: any) => { - const rawReturn = req.query.return_to; - const returnTo = - typeof rawReturn === 'string' && rawReturn.startsWith('/') && !rawReturn.startsWith('//') - ? rawReturn - : '/'; - setRenewFailedCookie(req, res); - return res.redirect(returnTo); -}); - -// ─── Iframe renew completion (postMessage to parent) ──────────────── -// The frontend opens a hidden iframe to /auth/login?renew=true&return_to=/auth/renew-done -// After the OIDC prompt=none dance, the iframe lands here and signals the parent. -router.get('/auth/renew-done', (req: any, res: any) => { - const success = !!req.user; - res.set('Cache-Control', 'no-store'); - return res.send( - ``, - ); -}); - // ─── Logout ────────────────────────────────────────────────────────── -router.post('/auth/logout', (req: any, res: any) => { +router.post('/auth/logout', (req, res: any) => { // Clear local session - req.logout(() => { - // Set pp-lic to logged-out state. Must use the SAME domain scope the - // login marker was set with (callback/session-set), otherwise on the - // duqduq deploy (where isProd() is false) this 'pp-lo' is host-only and - // fails to overwrite the .duqduq.org-scoped 'pp-li-…' — leaving the user - // looking logged-in to silentReauthMiddleware, which then loops. - res.cookie('pp-lic', 'pp-lo', { - ...(isProd() && { domain: '.pubpub.org' }), - ...(isDuqDuq() && { domain: '.duqduq.org' }), - maxAge: 30 * 24 * 60 * 60 * 1000, - }); - - // Redirect to KF Auth's signout relay so the SSO session is also - // cleared. (The relay POSTs to better-auth's sign-out — a plain GET - // redirect to /api/auth/sign-out would be rejected as POST-only.) - const returnUrl = `${process.env.APP_URL || 'http://localhost:9876'}/`; - return res.redirect( - `${OIDC_ISSUER_URL}/auth/signout?redirect_uri=${encodeURIComponent(returnUrl)}`, - ); - }); + logout(req, res); + const returnUrl = `${process.env.APP_URL || 'http://localhost:9876'}/`; + return res.redirect( + `${OIDC_ISSUER_URL}/auth/signout?redirect_uri=${encodeURIComponent(returnUrl)}`, + ); }); // ─── Webhooks from KF Auth ────────────────────────────────────────── diff --git a/server/kf/oidc.server.ts b/server/kf/oidc.server.ts index c1b49f4c5c..e145d24893 100644 --- a/server/kf/oidc.server.ts +++ b/server/kf/oidc.server.ts @@ -40,6 +40,7 @@ interface OIDCDiscovery { authorization_endpoint: string; token_endpoint: string; userinfo_endpoint: string; + introspection_endpoint?: string; jwks_uri?: string; } @@ -52,7 +53,7 @@ async function discover(): Promise { discoveryPromise = (async () => { const url = `${OIDC_ISSUER_INTERNAL_URL}/.well-known/openid-configuration`; - const res = await fetch(url); + const res = await fetchWithTimeout(url); if (!res.ok) { throw new Error( `OIDC discovery failed: ${res.status} from ${url}. ` + @@ -89,6 +90,25 @@ function internalEndpoint(discoveredUrl: string): string { return url.toString(); } +// Server-to-server calls to kf-auth MUST be bounded: without a timeout a slow +// or unreachable kf-auth hangs the request indefinitely (and, on the session +// refresh path, the in-process single-flight makes every other request for that +// session hang on the same pending promise). 8s is generous for a token call. +const KF_FETCH_TIMEOUT_MS = 8000; + +async function fetchWithTimeout( + input: string, + init?: Parameters[1], +): Promise { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), KF_FETCH_TIMEOUT_MS); + try { + return await fetch(input, { ...init, signal: controller.signal }); + } finally { + clearTimeout(timer); + } +} + // --- Symmetric encryption (AES-256-GCM) --- /** Derive a 32-byte key from the client secret for AES-256-GCM. */ @@ -161,7 +181,11 @@ export async function buildAuthorizeUrl( client_id: OIDC_CLIENT_ID, redirect_uri: REDIRECT_URI, response_type: 'code', - scope: 'openid profile email', + // offline_access makes kf-auth issue a refresh token (Better Auth gates + // refresh-token issuance on this scope). PubPub is a confidential, + // server-side client, so it renews the session via the refresh_token + // grant server-to-server — no browser redirect / third-party-cookie dance. + scope: 'openid profile email offline_access', state, code_challenge: codeChallenge, code_challenge_method: 'S256', @@ -216,7 +240,7 @@ export async function exchangeCode(code: string, codeVerifier: string): Promise< code_verifier: codeVerifier, }); - const res = await fetch(internalEndpoint(config.token_endpoint), { + const res = await fetchWithTimeout(internalEndpoint(config.token_endpoint), { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, body, @@ -230,6 +254,51 @@ export async function exchangeCode(code: string, codeVerifier: string): Promise< return res.json() as Promise; } +export interface IntrospectionResult { + /** kf-auth says the token (and its session) is live. */ + active: boolean; + /** kf-auth session id, present only while the session itself is live. */ + sid?: string; +} + +/** + * RFC 7662 token introspection of a refresh token (server-to-server). + * + * This is a READ-ONLY liveness check — unlike the refresh_token grant it does + * NOT rotate the token, so it's idempotent and safe to call concurrently from + * any number of instances. kf-auth returns `active: false` once the refresh + * token is revoked (our session.delete hook revokes it when the kf-auth session + * is revoked / the user is banned) or expired, and nulls `sid` when the backing + * session is gone. We treat "not active" OR "no sid" as dead. + * + * Throws on transient failure (network / timeout / non-2xx) — the caller keeps + * the session and retries on the next cycle (fail-open on a kf-auth blip). + */ +export async function introspectRefreshToken(refreshToken: string): Promise { + const config = await discover(); + const endpoint = + config.introspection_endpoint ?? config.token_endpoint.replace(/\/token$/, '/introspect'); + const body = new URLSearchParams({ + token: refreshToken, + token_type_hint: 'refresh_token', + client_id: OIDC_CLIENT_ID, + client_secret: OIDC_CLIENT_SECRET, + }); + + const res = await fetchWithTimeout(internalEndpoint(endpoint), { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body, + }); + + if (!res.ok) { + throw new Error(`Introspection failed: ${res.status} ${await res.text()}`); + } + + const json = (await res.json()) as { active?: boolean; sid?: string }; + return { active: json.active === true, sid: json.sid }; +} + // --- UserInfo --- export interface OIDCOrg { @@ -252,7 +321,7 @@ export interface OIDCUserInfo { export async function fetchUserInfo(accessToken: string): Promise { const config = await discover(); - const res = await fetch(internalEndpoint(config.userinfo_endpoint), { + const res = await fetchWithTimeout(internalEndpoint(config.userinfo_endpoint), { headers: { Authorization: `Bearer ${accessToken}` }, }); diff --git a/server/kf/sessionCheck.ts b/server/kf/sessionCheck.ts new file mode 100644 index 0000000000..828fdf32f3 --- /dev/null +++ b/server/kf/sessionCheck.ts @@ -0,0 +1,130 @@ +/** + * Server-side session liveness checks against kf-auth via RFC 7662 token + * introspection. + * + * PubPub is a confidential (server-side) OIDC client. It never calls kf-auth + * APIs with the user's token after login — it only needs to know "is this + * kf-auth session still alive?". So instead of redeeming the refresh token (the + * refresh_token grant rotates it, which is stateful and races across instances), + * it INTROSPECTS the refresh token: a read-only, idempotent check that is safe + * to call concurrently from any number of instances. The refresh token is + * therefore written once at login and never changes, so it lives happily on the + * express session blob — no side table, no row lock, no transaction. + * + * kf-auth reports the token inactive once it's revoked (the session.delete hook + * revokes the OAuth refresh tokens when a kf-auth session is revoked / the user + * is banned) or expired, at which point we tear the local session down. The + * `session.revoked` webhook still provides instant revocation; this is the + * backstop if a webhook is missed. + */ +import type { Request, Response } from 'express'; +import type { Session } from 'express-session'; + +import { promisify } from 'util'; + +import { logout } from 'server/utils/logout'; +import { isDevelopment, isDuqDuq } from 'utils/environment'; + +import { decryptPayload, encryptPayload, introspectRefreshToken } from './oidc.server'; + +// Worst-case revocation/ban latency if the webhook is missed. Short in dev / +// duqduq for fast testing; a few minutes in prod. Override with +// KF_REVALIDATE_MS for local testing. +export const KF_REVALIDATE_MS = process.env.KF_REVALIDATE_MS + ? Number(process.env.KF_REVALIDATE_MS) + : isDevelopment() + ? 20 * 1000 + : isDuqDuq() + ? 60 * 1000 + : 5 * 60 * 1000; + +// Back off this long after a transient (network / timeout / 5xx) check failure. +const TRANSIENT_RETRY_MS = 30 * 1000; + +/** + * Store the refresh token (encrypted) + kf session id on the session and + * schedule the first liveness check. Called at login (callback / session-set). + * The token is never rewritten afterwards (introspection doesn't rotate it), so + * keeping it on the session blob is safe. + */ +export function setKfSessionTokens( + req: Request, + opts: { refreshToken?: string | null; kfSessionId?: string | null }, +): void { + if (opts.kfSessionId) { + req.session.kfSessionId = opts.kfSessionId; + } + if (!opts.refreshToken) return; + req.session.kfRefreshToken = encryptPayload({ t: opts.refreshToken }); + req.session.kfNextCheck = Date.now() + KF_REVALIDATE_MS; +} + +// In-process single-flight: collapse concurrent checks for the same session +// (e.g. a page firing many parallel requests) into one introspection call. +const inFlight = new Map>(); + +/** + * Check the session against kf-auth if it's due. Returns true if the session is + * (still) valid, false if it was torn down. Never throws. + */ +export async function checkKfSession(req: Request, res: Response): Promise { + const session = req.session; + // Not an OIDC session (e.g. bearer-token API access) — nothing to check. + if (!session?.kfRefreshToken) return true; + if (Date.now() < (session.kfNextCheck ?? 0)) return true; // still fresh + + const sid = req.sessionID; + const existing = inFlight.get(sid); + if (existing) return existing; + + const p = doCheck(req, res).finally(() => inFlight.delete(sid)); + inFlight.set(sid, p); + return p; +} + +async function doCheck(req: Request, res: Response): Promise { + const session = req.session; + const decrypted = decryptPayload<{ t: string }>(session.kfRefreshToken ?? ''); + if (!decrypted?.t) { + teardown(req, res); + return false; + } + + try { + const { active, sid } = await introspectRefreshToken(decrypted.t); + // `active` goes false on revoke/expire; `sid` is nulled when the backing + // session is gone. Either means the kf-auth session no longer exists. + if (!active || !sid) { + teardown(req, res); + return false; + } + } catch { + // Transient (network / timeout / 5xx) — keep the session, retry shortly. + session.kfNextCheck = Date.now() + TRANSIENT_RETRY_MS; + await saveSession(session); + return true; + } + + session.kfNextCheck = Date.now() + KF_REVALIDATE_MS; + await saveSession(session); + return true; +} + +function teardown(req: Request, res: Response): void { + // logout() clears req.user and flips the pp-lic CDN marker to logged-out. + // req.user being cleared means subsequent requests skip the check entirely + // (the middleware only runs when req.user is set). + try { + logout(req, res); + } catch { + /* best-effort */ + } +} + +async function saveSession(session: Session): Promise { + try { + await promisify(session.save.bind(session))(); + } catch { + /* best-effort — a failed save just means we re-check sooner */ + } +} diff --git a/server/middleware/kfSessionCheck.ts b/server/middleware/kfSessionCheck.ts new file mode 100644 index 0000000000..1be1b910e6 --- /dev/null +++ b/server/middleware/kfSessionCheck.ts @@ -0,0 +1,30 @@ +import type { NextFunction, Request, Response } from 'express'; + +import { checkKfSession } from '../kf/sessionCheck'; + +/** + * Keeps the local session in sync with kf-auth's authority over it. + * + * For a logged-in OIDC session that's due, this introspects its refresh token + * against kf-auth (see sessionCheck.ts). If the kf-auth session was revoked or + * the user banned, the token reads as inactive and the local session is torn + * down (which also flips the `pp-lic` CDN marker to logged-out). + * + * This replaced the old browser-driven `prompt=none` silent re-auth, which + * could not work across registrable domains (kf-auth's SameSite=Lax session + * cookie is never sent on the cross-site iframe/redirect the renewal needed, + * and third-party-cookie protection blocks it regardless). + */ +export const kfSessionCheckMiddleware = () => { + return async (req: Request, res: Response, next: NextFunction) => { + if (!req.user) return next(); + + try { + await checkKfSession(req, res); + } catch { + // Never block a request on a revalidation hiccup — retry next time. + } + + next(); + }; +}; diff --git a/server/middleware/silentReauth.ts b/server/middleware/silentReauth.ts deleted file mode 100644 index 0cf0564152..0000000000 --- a/server/middleware/silentReauth.ts +++ /dev/null @@ -1,49 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; - -const PAGE_SKIP_PREFIXES = ['/auth', '/dist', '/static', '/service-worker', '/favicon']; - -/** - * Detects "was logged in, session expired" and triggers silent re-auth. - * - * - Page navigations (GET to non-asset paths): 302 redirect to OIDC prompt=none. - * - API requests (/api/*): 401 JSON with `error: 'sessionExpired'` so the - * frontend can renew via a hidden iframe and retry transparently. - * - * Uses the `pp-lic` CDN cookie (set at login, 30-day maxAge) to detect - * that the user was previously authenticated. A `pp-renew-failed` cookie - * acts as a circuit breaker to prevent redirect loops when kf-auth's - * session is also expired. - */ -export const silentReauthMiddleware = () => { - return (req: Request, res: Response, next: NextFunction) => { - if (req.user) return next(); - - // After logout it's set to 'pp-lo' — renewing would resurrect the session the user just deliberately ended. - const lic = req.cookies?.['pp-lic']; - if (typeof lic !== 'string' || !lic.startsWith('pp-li-')) return next(); - - // Circuit breaker: recently tried and failed — skip - if (req.cookies?.['pp-renew-failed']) return next(); - - // ── API requests: tell the frontend to renew ── - if (req.path.startsWith('/api')) { - return res.status(401).json({ error: 'sessionExpired' }); - } - - // ── Page navigations: redirect through OIDC prompt=none ── - if (req.method !== 'GET') return next(); - if (PAGE_SKIP_PREFIXES.some((p) => req.path.startsWith(p))) return next(); - - // This 302 carries no Set-Cookie, so Fastly would otherwise cache it - // under the per-`pp-lic` cache key (vcl_hash only mixes connect.sid in - // for /api routes). A cached "go reauth" redirect would then be served - // even after the user has a valid session again — an infinite loop the - // session cookie can't bust. Mark it private/no-store so the edge - // passes it through (Fastly return(pass)es on `Cache-Control ~ private`). - res.set('Cache-Control', 'private, no-store'); - res.set('Surrogate-Control', 'no-store'); - - const returnTo = req.originalUrl; - return res.redirect(`/auth/login?renew=true&return_to=${encodeURIComponent(returnTo)}`); - }; -}; diff --git a/server/server.ts b/server/server.ts index a04ae7a350..079bed4f2f 100755 --- a/server/server.ts +++ b/server/server.ts @@ -35,7 +35,7 @@ if (env.NODE_ENV !== 'test') { import { communityBanGuard } from './middleware/communityBanGuard'; import { deduplicateSlash } from './middleware/deduplicateSlash'; -import { silentReauthMiddleware } from './middleware/silentReauth'; +import { kfSessionCheckMiddleware } from './middleware/kfSessionCheck'; import { blocklistMiddleware } from './utils/blocklist'; import './hooks'; @@ -156,16 +156,14 @@ appRouter.use( secure: env.NODE_ENV === 'production', maxAge: env.NODE_ENV === 'production' - ? isDuqDuq() - ? 1 * 60 * 1000 - : 15 * 60 * 1000 - : 10_000, // 1min duqduq, 15m prod, 10s dev for testing + ? 30 * 24 * 60 * 60 * 1000 // 30d prod + duqduq + : 24 * 60 * 60 * 1000, // 1d dev }, }), ); appRouter.use((req, res, next) => { - /* If on a platform domain, set the session cookie to be accessible */ + /* If on a pubpub/duqduq domain, set the session cookie to be accessible */ /* across all subdomains to maintain login. Especially important when */ /* creating communities. */ const hostname = req.headers.communityhostname || req.hostname; @@ -189,6 +187,7 @@ appRouter.use((req, res, next) => { /* ------------------- */ appRouter.use(passport.initialize()); appRouter.use(passport.session()); + passport.use(User.createStrategy()); passport.use('zotero', zoteroAuthStrategy()); passport.use('bearer', bearerStrategy()); @@ -290,7 +289,7 @@ appRouter.use(authTokenMiddleware); appRouter.use(purgeMiddleware(schedulePurge)); appRouter.use(readOnlyMiddleware()); -appRouter.use(silentReauthMiddleware()); +appRouter.use(kfSessionCheckMiddleware()); appRouter.use(communityBanGuard()); const { customScript: _, ...contractWithoutCustomScript } = contract; diff --git a/types/global.d.ts b/types/global.d.ts index 6527e96067..d3a6eefb90 100644 --- a/types/global.d.ts +++ b/types/global.d.ts @@ -1,4 +1,4 @@ -import { UserWithPrivateFields } from './user'; +import type { UserWithPrivateFields } from './user'; export {}; @@ -9,3 +9,11 @@ declare global { } } } + +declare module 'express-session' { + interface SessionData { + kfSessionId?: string; + kfRefreshToken?: string; + kfNextCheck?: number; + } +}