Phase 3 validated a stored ID token hint against the currently configured issuer, but the oversize path never got that check: an ID token above the 3.9KB cookie limit was stored as the bare string 'sso', which collapsed to an undefined hint at logout and skipped validation entirely. Changing the issuer while such a session was live bounced the user to the new IdP on logout. Stores sso.<base64url(issuer)> instead and moves all marker interpretation into buildEndSessionUrl: raw ID token -> iss/aud-validated hint, issuer-tagged marker -> round-trip without a hint, anything else -> no round-trip. Every branch fails closed. Refs #798.
This commit is contained in:
@@ -187,7 +187,7 @@ describe('OIDC logout-to-IdP (#798 phase 3)', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('stores a bare marker for oversized ID tokens; logout still round-trips, without a hint', async () => {
|
||||
it('stores an issuer-tagged marker for oversized ID tokens; logout still round-trips, without a hint', async () => {
|
||||
idp.setNextUser({
|
||||
sub: 'logout-sub-5',
|
||||
email: '[email protected]',
|
||||
@@ -198,7 +198,10 @@ describe('OIDC logout-to-IdP (#798 phase 3)', () => {
|
||||
const cbRes = await ssoRoundTrip();
|
||||
const cookie = idTokenCookie(cbRes);
|
||||
expect(cookie).toBeTruthy();
|
||||
expect(decodeURIComponent(cookie.replace('oidc_id_token=', ''))).toBe('sso');
|
||||
// Issuer-tagged marker, not the (oversized) token itself.
|
||||
const marker = decodeURIComponent(cookie.replace('oidc_id_token=', ''));
|
||||
expect(marker.startsWith('sso.')).toBe(true);
|
||||
expect(Buffer.from(marker.split('.')[1], 'base64url').toString('utf8')).toBe(idp.issuer);
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/auth/logout')
|
||||
@@ -210,6 +213,15 @@ describe('OIDC logout-to-IdP (#798 phase 3)', () => {
|
||||
expect(url.searchParams.get('client_id')).toBe(idp.clientId);
|
||||
});
|
||||
|
||||
it('skips the round-trip for an oversized-token marker from a DIFFERENT issuer', async () => {
|
||||
const foreignMarker = `sso.${Buffer.from('http://other-idp.example').toString('base64url')}`;
|
||||
const res = await request(app)
|
||||
.post('/api/auth/logout')
|
||||
.set('Cookie', `oidc_id_token=${foreignMarker}`)
|
||||
.expect(200);
|
||||
expect(res.body.ssoLogoutUrl).toBeUndefined();
|
||||
});
|
||||
|
||||
it('a fresh local-password login clears a stale SSO marker', async () => {
|
||||
const role = await db('roles').where({ name: 'admin' }).first();
|
||||
await db('admin_users').insert({
|
||||
|
||||
@@ -353,10 +353,10 @@ router.post('/logout', async (req, res) => {
|
||||
const oidcIdToken = req.cookies?.[OIDC_ID_TOKEN_COOKIE];
|
||||
if (oidcIdToken) {
|
||||
res.clearCookie(OIDC_ID_TOKEN_COOKIE, oidcIdTokenClearOptions());
|
||||
// The bare 'sso' marker (oversized ID token at login) still triggers
|
||||
// the round-trip, just without an id_token_hint.
|
||||
const hint = oidcIdToken.split('.').length === 3 ? oidcIdToken : undefined;
|
||||
ssoLogoutUrl = await require('../services/oidcService').buildEndSessionUrl(hint);
|
||||
// The service interprets the marker itself: raw ID token → hint
|
||||
// (iss/aud-validated), issuer-tagged 'sso.<b64>' marker (oversized
|
||||
// token) → round-trip without a hint, unknown/foreign origin → null.
|
||||
ssoLogoutUrl = await require('../services/oidcService').buildEndSessionUrl(oidcIdToken);
|
||||
}
|
||||
|
||||
res.json({ message: 'Logged out successfully', ...(ssoLogoutUrl ? { ssoLogoutUrl } : {}) });
|
||||
@@ -1065,11 +1065,14 @@ router.get('/admin/sso/callback', async (req, res) => {
|
||||
|
||||
// Cookie limit is 4KB — an oversized ID token (huge group lists) can't
|
||||
// be stored, but the SSO-session marker must survive or logout-to-IdP
|
||||
// silently turns off for exactly those users. Store the bare marker
|
||||
// 'sso' instead; logout then sends the end-session request without an
|
||||
// id_token_hint (costs one IdP confirmation click).
|
||||
// silently turns off for exactly those users. Store an issuer-tagged
|
||||
// marker instead; logout then sends the end-session request without an
|
||||
// id_token_hint (costs one IdP confirmation click), and the tag still
|
||||
// lets buildEndSessionUrl refuse the round-trip after an issuer change.
|
||||
if (idToken) {
|
||||
const cookieValue = idToken.length <= 3900 ? idToken : 'sso';
|
||||
const cookieValue = idToken.length <= 3900
|
||||
? idToken
|
||||
: oidcService.buildOversizeSsoMarker(claims.iss);
|
||||
res.cookie(OIDC_ID_TOKEN_COOKIE, cookieValue, oidcIdTokenCookieOptions(res));
|
||||
}
|
||||
|
||||
|
||||
@@ -374,28 +374,48 @@ function decodeJwtPayload(token) {
|
||||
}
|
||||
}
|
||||
|
||||
async function buildEndSessionUrl(idTokenHint) {
|
||||
/**
|
||||
* Marker for SSO sessions whose ID token is too large for a cookie:
|
||||
* `sso.<base64url(issuer)>`. Keeps the issuer identity so /logout can still
|
||||
* validate the marker against the current config — a bare marker would
|
||||
* round-trip to the wrong IdP after an issuer change.
|
||||
*/
|
||||
function buildOversizeSsoMarker(issuer) {
|
||||
return `sso.${Buffer.from(String(issuer || '')).toString('base64url')}`;
|
||||
}
|
||||
|
||||
async function buildEndSessionUrl(sessionMarker) {
|
||||
try {
|
||||
const cfg = await getOidcConfig();
|
||||
if (!cfg.enabled || !cfg.logoutFromIdp || !isConfigured(cfg)) return null;
|
||||
const { client, issuerMetadata } = await getClient(cfg);
|
||||
if (!issuerMetadata.end_session_endpoint) return null;
|
||||
|
||||
// The hint comes from a cookie minted at login — the OIDC config may
|
||||
// have changed since. A token from a DIFFERENT issuer means the session
|
||||
// belongs to another IdP entirely: skip the round-trip (ending the new
|
||||
// IdP's session would be wrong, and providers that validate the hint
|
||||
// would strand the user on an error page). Same issuer but a different
|
||||
// client (aud): the hint is unusable, but the browser's IdP session is
|
||||
// real — round-trip without the hint.
|
||||
let hint = idTokenHint;
|
||||
if (hint) {
|
||||
const payload = decodeJwtPayload(hint);
|
||||
if (!payload || payload.iss !== issuerMetadata.issuer) return null;
|
||||
const audMatches = Array.isArray(payload.aud)
|
||||
? payload.aud.includes(cfg.clientId)
|
||||
: payload.aud === cfg.clientId;
|
||||
if (!audMatches) hint = undefined;
|
||||
// The marker comes from a cookie minted at login — the OIDC config may
|
||||
// have changed since. A session from a DIFFERENT issuer must not be
|
||||
// round-tripped: ending the newly configured IdP's session would be
|
||||
// wrong, and providers that validate the hint would strand the user on
|
||||
// an error page. Same issuer but a different client (aud): the hint is
|
||||
// unusable, but the browser's IdP session is real — round-trip without
|
||||
// the hint. A marker whose origin can't be determined is skipped.
|
||||
let hint;
|
||||
if (sessionMarker) {
|
||||
const parts = String(sessionMarker).split('.');
|
||||
if (parts.length === 3) {
|
||||
// Raw ID token.
|
||||
const payload = decodeJwtPayload(sessionMarker);
|
||||
if (!payload || payload.iss !== issuerMetadata.issuer) return null;
|
||||
const audMatches = Array.isArray(payload.aud)
|
||||
? payload.aud.includes(cfg.clientId)
|
||||
: payload.aud === cfg.clientId;
|
||||
if (audMatches) hint = sessionMarker;
|
||||
} else if (parts.length === 2 && parts[0] === 'sso') {
|
||||
// Oversized-token marker — issuer identity only, never a hint.
|
||||
const iss = Buffer.from(parts[1], 'base64url').toString('utf8');
|
||||
if (iss !== issuerMetadata.issuer) return null;
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
const postLogoutRedirectUri = await getPostLogoutRedirectUri();
|
||||
@@ -732,6 +752,7 @@ module.exports = {
|
||||
buildAuthorizationRequest,
|
||||
handleCallback,
|
||||
buildEndSessionUrl,
|
||||
buildOversizeSsoMarker,
|
||||
resolveAdminFromClaims,
|
||||
saveOidcSettings,
|
||||
invalidateDiscoveryCache,
|
||||
|
||||
Reference in New Issue
Block a user