diff --git a/backend/__tests__/integration/oidcLogout.test.js b/backend/__tests__/integration/oidcLogout.test.js index f03ca407..d18ccfdf 100644 --- a/backend/__tests__/integration/oidcLogout.test.js +++ b/backend/__tests__/integration/oidcLogout.test.js @@ -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: 'logout5@example.com', @@ -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({ diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 3936a1f7..ded3c862 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -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.' 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)); } diff --git a/backend/src/services/oidcService.js b/backend/src/services/oidcService.js index 2d770895..11d02146 100644 --- a/backend/src/services/oidcService.js +++ b/backend/src/services/oidcService.js @@ -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.`. 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,