diff --git a/server/src/auth/session.service.js b/server/src/auth/session.service.js index ca6cc68..af396bc 100644 --- a/server/src/auth/session.service.js +++ b/server/src/auth/session.service.js @@ -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. diff --git a/server/test/session.test.js b/server/test/session.test.js index 9965ae0..9e0753e 100644 --- a/server/test/session.test.js +++ b/server/test/session.test.js @@ -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)