Fix SSO flow-token / session type confusion (#32)
sessionFromDecoded validated sessions with a blocklist — it rejected a
token only when `decoded.stage` was present (the TOTP challenge). Because
every JWT is signed with the same JWT_SECRET and distinguished only by
claims, the SSO transaction cookie (sso_tx, which carries kind:'sso_tx'
and id:'sso' but no stage) passed validation and was accepted as a bogus
{ userId:'sso' } session.
requireAuth's DB re-load blocked protected admin routes, but non-DB
identity checks were fooled — notably siteMode's maintenance-preview
bypass, which trusts any truthy getUserFromRequest. An attacker could
start an SSO flow to obtain an sso_tx cookie and replay it as the auth
cookie / Bearer token to bypass the maintenance gate. The broader risk
was latent: any future code path trusting attachSession/getUserFromRequest
without a DB round-trip inherited an auth bypass.
Make session validation positively typed: real sessions are now stamped
with typ:'session' (createSession + mintMobileTokens), and
sessionFromDecoded accepts a token only when that marker is present. As
belt-and-suspenders it also rejects any token carrying a non-session
marker (stage || kind). Flow/challenge tokens are never stamped, so they
can no longer be mistaken for sessions.
Note: existing web cookie sessions predating this change lack the typ
claim and will be rejected once — users re-login. Mobile clients recover
automatically on next refresh.
Adds regression tests: the sso_tx flow token and a bare identity token
are both rejected by validateSession / decodeIdentity / getUserFromRequest.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -36,10 +36,24 @@ const log = require('../utils/logger')('session')
|
||||
// authenticated without changing this module per provider.
|
||||
const AUTH_METHODS = ['local', 'totp', 'mobile', 'google', 'discord', 'oidc', 'sso']
|
||||
|
||||
// The claim that positively marks a token as a real, full session. Every JWT in
|
||||
// the app is signed with the same secret and is distinguished only by claims, so
|
||||
// a session must be identified by what it *is* (typ === 'session'), never by the
|
||||
// mere absence of some other marker. Only the session-minting paths below stamp
|
||||
// it; flow/challenge tokens (the TOTP challenge, the SSO transaction cookie) do
|
||||
// not, so — even though they verify against the same secret — they can never be
|
||||
// mistaken for a session. See issue #32 (sso_tx token-type confusion).
|
||||
const SESSION_TYP = 'session'
|
||||
|
||||
// Build a Session object from a decoded JWT payload. Returns null for anything
|
||||
// that is not a full session (e.g. a stage-tagged TOTP challenge token).
|
||||
// that is not a full session. Validation is positively typed: a token qualifies
|
||||
// only if it was explicitly minted as a session. As belt-and-suspenders we also
|
||||
// reject any token carrying a non-session marker (stage = TOTP challenge, kind =
|
||||
// SSO transaction), so a future minting path that forgets to omit those still
|
||||
// can't produce an accepted session.
|
||||
function sessionFromDecoded(decoded, now = Date.now()) {
|
||||
if (!decoded || decoded.stage) return null
|
||||
if (!decoded || decoded.typ !== SESSION_TYP) return null
|
||||
if (decoded.stage || decoded.kind) return null
|
||||
return {
|
||||
sessionId: decoded.jti || null,
|
||||
userId: decoded.id,
|
||||
@@ -61,7 +75,7 @@ function sessionFromDecoded(decoded, now = Date.now()) {
|
||||
function createSession(user, authMethod = 'local') {
|
||||
const method = AUTH_METHODS.includes(authMethod) ? authMethod : 'local'
|
||||
const sessionId = crypto.randomUUID()
|
||||
const raw = token.signToken(user, { authMethod: method, jti: sessionId })
|
||||
const raw = token.signToken(user, { authMethod: method, jti: sessionId, typ: SESSION_TYP })
|
||||
const session = sessionFromDecoded(token.verifyToken(raw))
|
||||
log.info('session created', { userId: user.id, username: user.username, authMethod: method, sessionId })
|
||||
return { token: raw, session }
|
||||
@@ -129,7 +143,7 @@ function mintMobileTokens(user, meta = {}, now = Date.now()) {
|
||||
const sessionId = crypto.randomUUID()
|
||||
const accessToken = token.signToken(
|
||||
user,
|
||||
{ authMethod: 'mobile', jti: sessionId },
|
||||
{ authMethod: 'mobile', jti: sessionId, typ: SESSION_TYP },
|
||||
{ expiresIn: MOBILE_ACCESS_TTL },
|
||||
)
|
||||
// 256 bits of entropy, url-safe. Opaque — carries no claims.
|
||||
|
||||
@@ -10,6 +10,7 @@ const { test, after } = require('node:test')
|
||||
const assert = require('node:assert/strict')
|
||||
|
||||
const sessionService = require('../src/auth/session.service')
|
||||
const ssoState = require('../src/auth/ssoState')
|
||||
const authFacade = require('../src/utils/auth')
|
||||
const revokedSessions = require('../src/model/revokedSessions/revokedSessions.model')
|
||||
const usersModel = require('../src/model/users/users.model')
|
||||
@@ -73,6 +74,28 @@ test('a partial (TOTP challenge) token is NOT a valid session', () => {
|
||||
assert.equal(sessionService.decodeIdentity(challenge), null)
|
||||
})
|
||||
|
||||
test('an SSO transaction (sso_tx) flow token is NOT a valid session (issue #32)', () => {
|
||||
// The sso_tx cookie is a JWT signed with the same secret as sessions, carrying
|
||||
// kind:'sso_tx' and id:'sso' but no `stage`. Before the fix it passed the
|
||||
// blocklist check and validated as a bogus { userId:'sso' } session, which fooled
|
||||
// non-DB identity checks (e.g. siteMode's maintenance-preview bypass).
|
||||
const { txToken } = ssoState.createTx({ provider: 'google', mode: 'login' })
|
||||
assert.equal(sessionService.validateSession(reqWithCookie(txToken)), null)
|
||||
assert.equal(sessionService.validateSession(reqWithBearer(txToken)), null)
|
||||
assert.equal(sessionService.decodeIdentity(txToken), null)
|
||||
assert.equal(sessionService.validateBearerToken(txToken), null)
|
||||
// And the historical facade used by siteMode must report no user.
|
||||
assert.equal(authFacade.getUserFromRequest(reqWithCookie(txToken)), null)
|
||||
})
|
||||
|
||||
test('a bare identity token with no session marker is NOT a valid session', () => {
|
||||
// A JWT carrying only { id, username, role } (e.g. a legacy token, or one minted
|
||||
// for some other purpose) must not validate: sessions are positively typed.
|
||||
const bare = authFacade.signToken(USER)
|
||||
assert.equal(sessionService.validateSession(reqWithCookie(bare)), null)
|
||||
assert.equal(sessionService.decodeIdentity(bare), null)
|
||||
})
|
||||
|
||||
test('upgradeSessionAfterTotp accepts a challenge and rejects a session token', () => {
|
||||
const challenge = sessionService.createPartialSession(USER)
|
||||
const decoded = sessionService.upgradeSessionAfterTotp(challenge)
|
||||
|
||||
Reference in New Issue
Block a user