fix(admin): improve admin session cookie and response handling

Signed-off-by: ABHAY PANDEY <pandeyabhay967@gmail.com>

Author: Ferryx349

src/admin/password-admin-auth-provider.ts

@@ -5,40 +5,40 @@ import { Settings } from '../@types/settings'
 import { adminLoginBodySchema } from '../schemas/admin-login-schema'
 import { verifyAdminPasswordHash, verifyPlaintextPassword } from '../utils/admin-password'
 import {
+  buildAdminSessionCookieHeader,
   createAdminSessionToken,
   getAdminSessionTokenFromRequest,
   isValidAdminSessionToken,
   parseAdminSessionToken,
+  resolveAdminSessionTtlSeconds,
 } from '../utils/admin-session'
 import { validateSchema } from '../utils/validation'
 
-const DEFAULT_SESSION_TTL_SECONDS = 86400
-
 export class PasswordAdminAuthProvider implements IAdminAuthProvider {
   public constructor(private readonly settings: () => Settings) {}
 
   public async handleLogin(request: Request, response: Response): Promise<void> {
     const validation = validateSchema(adminLoginBodySchema)(request.body)
     if (validation.error) {
-      response.status(400).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Invalid request' }))
+      response.status(400).setHeader('content-type', 'application/json').send({ error: 'Invalid request' })
       return
     }
 
     if (!this.verifyPassword(validation.value.password)) {
-      response.status(401).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Unauthorized' }))
+      response.status(401).setHeader('content-type', 'application/json').send({ error: 'Unauthorized' })
       return
     }
 
     const currentSettings = this.settings()
-    const ttl = currentSettings.admin?.sessionTtlSeconds ?? DEFAULT_SESSION_TTL_SECONDS
-    const expiresAt = Math.floor(Date.now() / 1000) + ttl
+    const sessionTtlSeconds = resolveAdminSessionTtlSeconds(currentSettings.admin?.sessionTtlSeconds)
+    const expiresAt = Math.floor(Date.now() / 1000) + sessionTtlSeconds
     const token = createAdminSessionToken(expiresAt)
 
     response
       .status(200)
       .setHeader('content-type', 'application/json')
-      .setHeader('Set-Cookie', `admin_session=${token}; Path=/admin; HttpOnly; SameSite=Strict; Max-Age=${ttl}`)
-      .send(JSON.stringify({ authenticated: true, expiresAt }))
+      .setHeader('Set-Cookie', buildAdminSessionCookieHeader(request, currentSettings, token, sessionTtlSeconds))
+      .send({ authenticated: true, expiresAt })
   }
 
   public isRequestAuthenticated(request: Request): boolean {

src/controllers/admin/get-health-controller.ts

@@ -6,6 +6,6 @@ import { collectAdminHealthSnapshot } from '../../utils/admin-health'
 export class GetAdminHealthController implements IController {
   public async handleRequest(_request: Request, response: Response): Promise<void> {
     const health = await collectAdminHealthSnapshot()
-    response.status(200).setHeader('content-type', 'application/json').send(JSON.stringify(health))
+    response.status(200).setHeader('content-type', 'application/json').send(health)
   }
 }

src/controllers/admin/get-session-controller.ts

@@ -7,16 +7,9 @@ export class GetAdminSessionController implements IController {
   public constructor(private readonly authProvider: IAdminAuthProvider) {}
 
   public async handleRequest(request: Request, response: Response): Promise<void> {
-    if (!this.authProvider.isRequestAuthenticated(request)) {
-      response.status(401).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Unauthorized' }))
-      return
-    }
-
-    response.status(200).setHeader('content-type', 'application/json').send(
-      JSON.stringify({
-        authenticated: true,
-        expiresAt: this.authProvider.getSessionExpiresAt(request),
-      }),
-    )
+    response.status(200).setHeader('content-type', 'application/json').send({
+      authenticated: true,
+      expiresAt: this.authProvider.getSessionExpiresAt(request),
+    })
   }
 }

src/handlers/request-handlers/admin-rate-limit-middleware.ts

@@ -5,7 +5,7 @@ import { rateLimiterFactory } from '../../factories/rate-limiter-factory'
 import { isAdminRateLimited } from '../../utils/admin-rate-limit'
 
 const sendTooManyRequests = (response: Response) => {
-  response.status(429).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Too many requests' }))
+  response.status(429).setHeader('content-type', 'application/json').send({ error: 'Too many requests' })
 }
 
 export const adminLoginRateLimitMiddleware = async (request: Request, response: Response, next: NextFunction) => {

src/routes/admin/index.ts

@@ -26,7 +26,7 @@ const requireAdminEnabled = (_request: Request, response: Response, next: NextFu
 
 const requireAdminAuth = (request: Request, response: Response, next: NextFunction) => {
   if (!createAdminAuthProvider().isRequestAuthenticated(request)) {
-    response.status(401).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Unauthorized' }))
+    response.status(401).setHeader('content-type', 'application/json').send({ error: 'Unauthorized' })
     return
   }
 

src/utils/admin-session.ts

@@ -1,7 +1,35 @@
 import { timingSafeEqual } from 'crypto'
+import { IncomingMessage } from 'http'
 
+import { Settings } from '../@types/settings'
+import { getPublicPathPrefix, isSecureRequest, joinPathPrefix } from './http'
 import { deriveFromSecret, hmacSha256 } from './secret'
 
+export const DEFAULT_ADMIN_SESSION_TTL_SECONDS = 86400
+
+export const resolveAdminSessionTtlSeconds = (
+  sessionTtlSeconds: number | undefined,
+  defaultTtlSeconds = DEFAULT_ADMIN_SESSION_TTL_SECONDS,
+): number => {
+  if (typeof sessionTtlSeconds === 'number' && Number.isFinite(sessionTtlSeconds) && sessionTtlSeconds > 0) {
+    return Math.floor(sessionTtlSeconds)
+  }
+
+  return defaultTtlSeconds
+}
+
+export const buildAdminSessionCookieHeader = (
+  request: IncomingMessage,
+  settings: Settings,
+  token: string,
+  maxAgeSeconds: number,
+): string => {
+  const cookiePath = joinPathPrefix(getPublicPathPrefix(request, settings), '/admin')
+  const secure = isSecureRequest(request, settings) ? '; Secure' : ''
+
+  return `admin_session=${token}; Path=${cookiePath}; HttpOnly; SameSite=Strict; Max-Age=${maxAgeSeconds}${secure}`
+}
+
 export const createAdminSessionToken = (expiresAt: number): string => {
   const signature = hmacSha256(deriveFromSecret('admin-session'), `${expiresAt}`).toString('hex')
   return `${expiresAt}.${signature}`

src/utils/http.ts

@@ -111,6 +111,23 @@ export const getPublicPathPrefix = (request: IncomingMessage, settings: Settings
   return getTrustedForwardedPathPrefix(request, settings) || getRelayUrlPathPrefix(settings.info?.relay_url)
 }
 
+export const isSecureRequest = (request: IncomingMessage, settings: Settings): boolean => {
+  if ('secure' in request && (request as { secure?: boolean }).secure === true) {
+    return true
+  }
+
+  const socketAddress = request.socket?.remoteAddress
+  if (typeof socketAddress !== 'string' || !isTrustedProxy(socketAddress, settings)) {
+    return false
+  }
+
+  const rawHeader = request.headers?.['x-forwarded-proto']
+  const rawProto = Array.isArray(rawHeader) ? rawHeader[0] : rawHeader
+  const proto = typeof rawProto === 'string' ? rawProto.split(',')[0].trim().toLowerCase() : ''
+
+  return proto === 'https'
+}
+
 export const joinPathPrefix = (prefix: string, path: string): string => {
   const normalizedPrefix = prefix.replace(/\/+$/, '')
   const normalizedPath = path.startsWith('/') ? path : `/${path}`

test/unit/utils/admin-session.spec.ts

@@ -1,6 +1,12 @@
 import { expect } from 'chai'
 
-import { createAdminSessionToken, getAdminSessionTokenFromRequest, isValidAdminSessionToken } from '../../../src/utils/admin-session'
+import {
+  buildAdminSessionCookieHeader,
+  createAdminSessionToken,
+  getAdminSessionTokenFromRequest,
+  isValidAdminSessionToken,
+  resolveAdminSessionTtlSeconds,
+} from '../../../src/utils/admin-session'
 
 describe('admin-session', () => {
   const originalSecret = process.env.SECRET
@@ -29,4 +35,48 @@ describe('admin-session', () => {
     expect(getAdminSessionTokenFromRequest('Bearer abc.def', undefined)).to.equal('abc.def')
     expect(getAdminSessionTokenFromRequest(undefined, 'admin_session=abc.def; other=value')).to.equal('abc.def')
   })
+
+  it('falls back to the default ttl for invalid sessionTtlSeconds values', () => {
+    expect(resolveAdminSessionTtlSeconds(undefined)).to.equal(86400)
+    expect(resolveAdminSessionTtlSeconds(0)).to.equal(86400)
+    expect(resolveAdminSessionTtlSeconds(-1)).to.equal(86400)
+    expect(resolveAdminSessionTtlSeconds(Number.NaN)).to.equal(86400)
+    expect(resolveAdminSessionTtlSeconds(3600)).to.equal(3600)
+  })
+
+  it('builds a secure admin session cookie behind a trusted https proxy', () => {
+    const cookie = buildAdminSessionCookieHeader(
+      {
+        headers: { 'x-forwarded-proto': 'https', 'x-forwarded-prefix': '/nostream' },
+        socket: { remoteAddress: '127.0.0.1' },
+      } as any,
+      {
+        info: { relay_url: 'wss://relay.example.com/nostream' },
+        network: { trustedProxies: ['127.0.0.1'] },
+      } as any,
+      'token-value',
+      3600,
+    )
+
+    expect(cookie).to.equal(
+      'admin_session=token-value; Path=/nostream/admin; HttpOnly; SameSite=Strict; Max-Age=3600; Secure',
+    )
+  })
+
+  it('omits Secure for local http requests', () => {
+    const cookie = buildAdminSessionCookieHeader(
+      {
+        headers: {},
+        socket: { remoteAddress: '127.0.0.1' },
+      } as any,
+      {
+        info: { relay_url: 'ws://localhost:8008' },
+        network: {},
+      } as any,
+      'token-value',
+      3600,
+    )
+
+    expect(cookie).to.equal('admin_session=token-value; Path=/admin; HttpOnly; SameSite=Strict; Max-Age=3600')
+  })
 })

test/unit/utils/http.spec.ts

@@ -1,7 +1,7 @@
 import { expect } from 'chai'
 import { IncomingMessage } from 'http'
 
-import { getPublicPathPrefix, getRemoteAddress, joinPathPrefix } from '../../../src/utils/http'
+import { getPublicPathPrefix, getRemoteAddress, isSecureRequest, joinPathPrefix } from '../../../src/utils/http'
 
 describe('getRemoteAddress', () => {
   const header = 'x-forwarded-for'
@@ -169,3 +169,50 @@ describe('joinPathPrefix', () => {
     expect(joinPathPrefix('/nostream', '/invoices')).to.equal('/nostream/invoices')
   })
 })
+
+describe('isSecureRequest', () => {
+  const settings = {
+    info: { relay_url: 'wss://relay.example.com' },
+    network: { trustedProxies: ['127.0.0.1'] },
+  } as any
+
+  it('returns true when express marks the request secure', () => {
+    expect(isSecureRequest({ secure: true, socket: { remoteAddress: 'client' } } as any, settings)).to.equal(true)
+  })
+
+  it('returns true for trusted x-forwarded-proto https', () => {
+    expect(
+      isSecureRequest(
+        {
+          headers: { 'x-forwarded-proto': 'https' },
+          socket: { remoteAddress: '127.0.0.1' },
+        } as any,
+        settings,
+      ),
+    ).to.equal(true)
+  })
+
+  it('returns false for untrusted x-forwarded-proto https', () => {
+    expect(
+      isSecureRequest(
+        {
+          headers: { 'x-forwarded-proto': 'https' },
+          socket: { remoteAddress: 'client' },
+        } as any,
+        settings,
+      ),
+    ).to.equal(false)
+  })
+
+  it('returns false for trusted http x-forwarded-proto', () => {
+    expect(
+      isSecureRequest(
+        {
+          headers: { 'x-forwarded-proto': 'http' },
+          socket: { remoteAddress: '127.0.0.1' },
+        } as any,
+        settings,
+      ),
+    ).to.equal(false)
+  })
+})