Skip to content

fix: remove prompt=none redirect loop, instead use refresh token checks #3638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tefkah merged 4 commits into from
Jun 18, 2026
Merged

fix: remove prompt=none redirect loop, instead use refresh token checks #3638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bda129c
fix: fix api reauth
tefkah Jun 16, 2026
494aa5d
fix: also make work for altcha
tefkah Jun 16, 2026
6ac0d2a
fix: don't drop headers
tefkah Jun 17, 2026
7661226
refactor: rely on refresh tokens instead, like we should have done fr…
tefkah Jun 18, 2026
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
7 changes: 6 additions & 1 deletion client/components/Altcha/Altcha.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down Expand Up @@ -40,9 +41,12 @@ const Altcha = forwardRef<AltchaRef, AltchaProps>((props, ref) => {
if (!loaded) return;
const w = widgetRef.current;
if (!w) return;
// Route the widget's challenge fetch through apiFetchRaw so it carries
// 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) => {
const e = ev as CustomEvent<{ payload?: string; state: string }>;
console.log('state changed', e.detail);

switch (e.detail.state) {
case 'error':
Expand Down Expand Up @@ -126,6 +130,7 @@ const Altcha = forwardRef<AltchaRef, AltchaProps>((props, ref) => {
delay={500}
ref={widgetRef as any}
challengeurl={challengeurl}
customfetch={apiFetchRaw}
{...(auto ? { auto } : {})}
floating="auto"
{...devAttrs}
Expand Down
29 changes: 21 additions & 8 deletions client/utils/apiFetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,35 @@ type HttpMethod = (typeof httpMethods)[number];

type ApiFetch = ApiFetchFn & { [K in HttpMethod]: HttpMethodApiFetchWrapper };

export const apiFetch = ((path, opts) => {
// ── Core fetch wrapper ──

/**
* 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<Response> {
return fetch(path, {
...opts,
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
...opts?.headers,
},
credentials: 'include',
}).then((response) => {
});
}

export const apiFetch = ((path, opts) => {
return apiFetchRaw(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'));
}
throw err;
});
const err = await response.json();

if (response.status === 423 && err?.error === 'readOnly') {
window.dispatchEvent(new CustomEvent('pubpub:readOnly'));
}
throw err;
}
return response.json();
});
Expand Down
200 changes: 28 additions & 172 deletions server/kf/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -40,6 +41,7 @@ import {
OIDC_ISSUER_URL,
} from './oidc.server';
import { provisionLocalUser } from './provisionLocalUser';
import { setKfSessionTokens } from './sessionCheck';
import {
handleSessionRevoked,
handleUserBanned,
Expand Down Expand Up @@ -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();
Expand All @@ -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);
});
Expand All @@ -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.');
}
Expand All @@ -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.');
Expand Down Expand Up @@ -263,26 +170,17 @@ 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)) {
const sessionToken = encryptPayload({
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)}`;
Expand All @@ -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}`, {
Expand All @@ -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}`);
}
Expand All @@ -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.');
}
Expand All @@ -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);
Expand All @@ -373,52 +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);
});

// ─── 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 ──────────────────────────────────────────
Expand Down
Loading
Loading