fix(session-cookie): migrate to __Host- prefix for secure contexts (#294)
* fix(session-cookie): migrate to __Host- prefix for secure contexts - Update session-cookie.ts to use __Host-mc-session for HTTPS requests - Add LEGACY_MC_SESSION_COOKIE_NAME for backward compatibility with HTTP - Add parseMcSessionCookieHeader() to parse both cookie names - Add isRequestSecure() helper to detect HTTPS requests - Update cookie options to enforce Secure, HttpOnly, SameSite=Strict - Update all call sites (login, logout, google, me, proxy, auth) - Update e2e tests to support both cookie names - Update documentation (README.md, SKILL.md, openapi.json) This addresses the high-priority TODO about migrating to the __Host- prefix for enhanced security. The __Host- prefix enforces Secure + Path=/ and prevents subdomain attacks. Legacy mc-session is still supported for HTTP contexts. * fix(tests): keep login-flow cookie name aligned with response - remove unreachable nullish expression in session cookie secure flag - use returned cookie pair in login-flow spec instead of forcing __Host- prefix --------- Co-authored-by: Nyk <0xnykcd@googlemail.com>
This commit is contained in:
parent
d2bbacbee3
commit
a86e939072
|
|
@ -308,7 +308,7 @@ Three auth methods, three roles:
|
|||
|
||||
| Method | Details |
|
||||
|--------|----------|
|
||||
| Session cookie | `POST /api/auth/login` sets `mc-session` (7-day expiry) |
|
||||
| Session cookie | `POST /api/auth/login` sets `__Host-mc-session` (7-day expiry) for HTTPS, `mc-session` for HTTP |
|
||||
| API key | `x-api-key` header matches `API_KEY` env var |
|
||||
| Google Sign-In | OAuth with admin approval workflow |
|
||||
|
||||
|
|
|
|||
2
SKILL.md
2
SKILL.md
|
|
@ -44,7 +44,7 @@ MC supports two auth methods:
|
|||
| Method | Header | Use Case |
|
||||
|--------|--------|----------|
|
||||
| API Key | `x-api-key: <key>` or `Authorization: Bearer <key>` | Agents, scripts, CI/CD |
|
||||
| Session cookie | `Cookie: mc-session=<token>` | Browser UI |
|
||||
| Session cookie | `Cookie: __Host-mc-session=<token>` (HTTPS) or `mc-session=<token>` (HTTP) | Browser UI |
|
||||
|
||||
**Roles (hierarchical):** `viewer` < `operator` < `admin`
|
||||
|
||||
|
|
|
|||
|
|
@ -1757,7 +1757,7 @@
|
|||
},
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "Login successful. Sets mc-session cookie.",
|
||||
"description": "Login successful. Sets __Host-mc-session cookie (HTTPS) or mc-session (HTTP).",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
|
|
@ -1775,7 +1775,7 @@
|
|||
"schema": {
|
||||
"type": "string"
|
||||
},
|
||||
"description": "mc-session cookie"
|
||||
"description": "__Host-mc-session cookie (secure HTTPS) or mc-session (HTTP legacy)"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
@ -7345,7 +7345,7 @@
|
|||
"sessionCookie": {
|
||||
"type": "apiKey",
|
||||
"in": "cookie",
|
||||
"name": "mc-session"
|
||||
"name": "__Host-mc-session"
|
||||
},
|
||||
"apiKey": {
|
||||
"type": "apiKey",
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { NextRequest, NextResponse } from 'next/server'
|
|||
import { createSession } from '@/lib/auth'
|
||||
import { getDatabase, logAuditEvent } from '@/lib/db'
|
||||
import { verifyGoogleIdToken } from '@/lib/google-auth'
|
||||
import { getMcSessionCookieOptions } from '@/lib/session-cookie'
|
||||
import { getMcSessionCookieName, getMcSessionCookieOptions, isRequestSecure } from '@/lib/session-cookie'
|
||||
import { loginLimiter } from '@/lib/rate-limit'
|
||||
|
||||
function upsertAccessRequest(input: {
|
||||
|
|
@ -100,10 +100,10 @@ export async function POST(request: NextRequest) {
|
|||
},
|
||||
})
|
||||
|
||||
const isSecureRequest = request.headers.get('x-forwarded-proto') === 'https'
|
||||
|| new URL(request.url).protocol === 'https:'
|
||||
const isSecureRequest = isRequestSecure(request)
|
||||
const cookieName = getMcSessionCookieName(isSecureRequest)
|
||||
|
||||
response.cookies.set('mc-session', token, {
|
||||
response.cookies.set(cookieName, token, {
|
||||
...getMcSessionCookieOptions({ maxAgeSeconds: expiresAt - Math.floor(Date.now() / 1000), isSecureRequest }),
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { NextResponse } from 'next/server'
|
||||
import { authenticateUser, createSession } from '@/lib/auth'
|
||||
import { logAuditEvent } from '@/lib/db'
|
||||
import { getMcSessionCookieOptions } from '@/lib/session-cookie'
|
||||
import { getMcSessionCookieName, getMcSessionCookieOptions, isRequestSecure } from '@/lib/session-cookie'
|
||||
import { loginLimiter } from '@/lib/rate-limit'
|
||||
import { logger } from '@/lib/logger'
|
||||
|
||||
|
|
@ -43,10 +43,10 @@ export async function POST(request: Request) {
|
|||
},
|
||||
})
|
||||
|
||||
const isSecureRequest = request.headers.get('x-forwarded-proto') === 'https'
|
||||
|| new URL(request.url).protocol === 'https:'
|
||||
const isSecureRequest = isRequestSecure(request)
|
||||
const cookieName = getMcSessionCookieName(isSecureRequest)
|
||||
|
||||
response.cookies.set('mc-session', token, {
|
||||
response.cookies.set(cookieName, token, {
|
||||
...getMcSessionCookieOptions({ maxAgeSeconds: expiresAt - Math.floor(Date.now() / 1000), isSecureRequest }),
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -1,13 +1,12 @@
|
|||
import { NextResponse } from 'next/server'
|
||||
import { destroySession, getUserFromRequest } from '@/lib/auth'
|
||||
import { logAuditEvent } from '@/lib/db'
|
||||
import { getMcSessionCookieOptions } from '@/lib/session-cookie'
|
||||
import { getMcSessionCookieName, getMcSessionCookieOptions, isRequestSecure, parseMcSessionCookieHeader } from '@/lib/session-cookie'
|
||||
|
||||
export async function POST(request: Request) {
|
||||
const user = getUserFromRequest(request)
|
||||
const cookieHeader = request.headers.get('cookie') || ''
|
||||
const match = cookieHeader.match(/(?:^|;\s*)mc-session=([^;]*)/)
|
||||
const token = match ? decodeURIComponent(match[1]) : null
|
||||
const token = parseMcSessionCookieHeader(cookieHeader)
|
||||
|
||||
if (token) {
|
||||
destroySession(token)
|
||||
|
|
@ -19,8 +18,10 @@ export async function POST(request: Request) {
|
|||
}
|
||||
|
||||
const response = NextResponse.json({ ok: true })
|
||||
response.cookies.set('mc-session', '', {
|
||||
...getMcSessionCookieOptions({ maxAgeSeconds: 0 }),
|
||||
const isSecureRequest = isRequestSecure(request)
|
||||
const cookieName = getMcSessionCookieName(isSecureRequest)
|
||||
response.cookies.set(cookieName, '', {
|
||||
...getMcSessionCookieOptions({ maxAgeSeconds: 0, isSecureRequest }),
|
||||
})
|
||||
|
||||
return response
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'
|
|||
import { getUserFromRequest, updateUser, requireRole, destroyAllUserSessions, createSession } from '@/lib/auth'
|
||||
import { logAuditEvent } from '@/lib/db'
|
||||
import { verifyPassword } from '@/lib/password'
|
||||
import { getMcSessionCookieOptions } from '@/lib/session-cookie'
|
||||
import { getMcSessionCookieName, getMcSessionCookieOptions, isRequestSecure } from '@/lib/session-cookie'
|
||||
import { logger } from '@/lib/logger'
|
||||
|
||||
export async function GET(request: Request) {
|
||||
|
|
@ -117,9 +117,9 @@ export async function PATCH(request: NextRequest) {
|
|||
// Issue a fresh session cookie after password change (old ones were just revoked)
|
||||
if (updates.password) {
|
||||
const { token, expiresAt } = createSession(user.id, ipAddress, userAgent, user.workspace_id ?? 1)
|
||||
const isSecureRequest = request.headers.get('x-forwarded-proto') === 'https'
|
||||
|| new URL(request.url).protocol === 'https:'
|
||||
response.cookies.set('mc-session', token, {
|
||||
const isSecureRequest = isRequestSecure(request)
|
||||
const cookieName = getMcSessionCookieName(isSecureRequest)
|
||||
response.cookies.set(cookieName, token, {
|
||||
...getMcSessionCookieOptions({ maxAgeSeconds: expiresAt - Math.floor(Date.now() / 1000), isSecureRequest }),
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { createHash, randomBytes, timingSafeEqual } from 'crypto'
|
|||
import { getDatabase } from './db'
|
||||
import { hashPassword, verifyPassword } from './password'
|
||||
import { logSecurityEvent } from './security-events'
|
||||
import { parseMcSessionCookieHeader } from './session-cookie'
|
||||
|
||||
// Plugin hook: extensions can register a custom API key resolver without modifying this file.
|
||||
type AuthResolverHook = (apiKey: string, agentName: string | null) => User | null
|
||||
|
|
@ -346,7 +347,7 @@ export function getUserFromRequest(request: Request): User | null {
|
|||
|
||||
// Check session cookie
|
||||
const cookieHeader = request.headers.get('cookie') || ''
|
||||
const sessionToken = parseCookie(cookieHeader, 'mc-session')
|
||||
const sessionToken = parseMcSessionCookieHeader(cookieHeader)
|
||||
if (sessionToken) {
|
||||
const user = validateSession(sessionToken)
|
||||
if (user) return { ...user, agent_name: agentName }
|
||||
|
|
@ -510,7 +511,3 @@ export function requireRole(
|
|||
return { user }
|
||||
}
|
||||
|
||||
function parseCookie(cookieHeader: string, name: string): string | null {
|
||||
const match = cookieHeader.match(new RegExp(`(?:^|;\\s*)${name}=([^;]*)`))
|
||||
return match ? decodeURIComponent(match[1]) : null
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,17 +1,27 @@
|
|||
import type { ResponseCookie } from 'next/dist/compiled/@edge-runtime/cookies'
|
||||
|
||||
// TODO: Migrate cookie name to use __Host- prefix for secure contexts.
|
||||
// The __Host- prefix enforces Secure + Path=/ and prevents subdomain attacks.
|
||||
// Migration path: add MC_SESSION_COOKIE_NAME usage to all callers
|
||||
// (proxy.ts, auth/login, auth/logout, auth/google, lib/auth.ts, tests)
|
||||
// then switch the default to use __Host- prefix when secure=true.
|
||||
export const MC_SESSION_COOKIE_NAME = 'mc-session'
|
||||
export const MC_SESSION_COOKIE_NAME = '__Host-mc-session'
|
||||
export const LEGACY_MC_SESSION_COOKIE_NAME = 'mc-session'
|
||||
const MC_SESSION_COOKIE_NAMES = [MC_SESSION_COOKIE_NAME, LEGACY_MC_SESSION_COOKIE_NAME] as const
|
||||
|
||||
export function getMcSessionCookieName(secure: boolean): string {
|
||||
// TODO: Enable __Host- prefix once all callers use this function.
|
||||
// When enabled: return secure ? '__Host-mc-session' : 'mc-session'
|
||||
void secure
|
||||
return MC_SESSION_COOKIE_NAME
|
||||
export function getMcSessionCookieName(isSecureRequest: boolean): string {
|
||||
return isSecureRequest ? MC_SESSION_COOKIE_NAME : LEGACY_MC_SESSION_COOKIE_NAME
|
||||
}
|
||||
|
||||
export function isRequestSecure(request: Request): boolean {
|
||||
return request.headers.get('x-forwarded-proto') === 'https'
|
||||
|| new URL(request.url).protocol === 'https:'
|
||||
}
|
||||
|
||||
export function parseMcSessionCookieHeader(cookieHeader: string): string | null {
|
||||
if (!cookieHeader) return null
|
||||
for (const cookieName of MC_SESSION_COOKIE_NAMES) {
|
||||
const match = cookieHeader.match(new RegExp(`(?:^|;\\s*)${cookieName}=([^;]*)`))
|
||||
if (match) {
|
||||
return decodeURIComponent(match[1])
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function envFlag(name: string): boolean | undefined {
|
||||
|
|
@ -25,23 +35,13 @@ function envFlag(name: string): boolean | undefined {
|
|||
|
||||
export function getMcSessionCookieOptions(input: { maxAgeSeconds: number; isSecureRequest?: boolean }): Partial<ResponseCookie> {
|
||||
const secureEnv = envFlag('MC_COOKIE_SECURE')
|
||||
// Explicit env wins. Otherwise auto-detect: only set secure if request came over HTTPS.
|
||||
// Falls back to NODE_ENV=production when no request hint is available.
|
||||
const secure = secureEnv ?? input.isSecureRequest ?? process.env.NODE_ENV === 'production'
|
||||
|
||||
// Strict is safest for this app (same-site UI + API), but allow override for edge cases.
|
||||
const sameSiteRaw = (process.env.MC_COOKIE_SAMESITE || 'strict').toLowerCase()
|
||||
const sameSite: ResponseCookie['sameSite'] =
|
||||
sameSiteRaw === 'lax' ? 'lax' :
|
||||
sameSiteRaw === 'none' ? 'none' :
|
||||
'strict'
|
||||
|
||||
return {
|
||||
httpOnly: true,
|
||||
secure,
|
||||
sameSite,
|
||||
sameSite: 'strict',
|
||||
maxAge: input.maxAgeSeconds,
|
||||
path: '/',
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import crypto from 'node:crypto'
|
|||
import os from 'node:os'
|
||||
import { NextResponse } from 'next/server'
|
||||
import type { NextRequest } from 'next/server'
|
||||
import { MC_SESSION_COOKIE_NAME, LEGACY_MC_SESSION_COOKIE_NAME } from '@/lib/session-cookie'
|
||||
|
||||
/** Constant-time string comparison using Node.js crypto. */
|
||||
function safeCompare(a: string, b: string): boolean {
|
||||
|
|
@ -189,7 +190,7 @@ export function proxy(request: NextRequest) {
|
|||
}
|
||||
|
||||
// Check for session cookie
|
||||
const sessionToken = request.cookies.get('mc-session')?.value
|
||||
const sessionToken = request.cookies.get(MC_SESSION_COOKIE_NAME)?.value || request.cookies.get(LEGACY_MC_SESSION_COOKIE_NAME)?.value
|
||||
|
||||
// API routes: accept session cookie OR API key
|
||||
if (pathname.startsWith('/api/')) {
|
||||
|
|
|
|||
|
|
@ -45,7 +45,7 @@ test.describe('Login Flow', () => {
|
|||
|
||||
const cookies = res.headers()['set-cookie']
|
||||
expect(cookies).toBeDefined()
|
||||
expect(cookies).toContain('mc-session')
|
||||
expect(cookies).toMatch(/(__Host-)?mc-session/)
|
||||
})
|
||||
|
||||
test('login API rejects wrong password', async ({ request }) => {
|
||||
|
|
@ -66,15 +66,16 @@ test.describe('Login Flow', () => {
|
|||
|
||||
// Extract session cookie from Set-Cookie header
|
||||
const setCookie = loginRes.headers()['set-cookie'] || ''
|
||||
const match = setCookie.match(/mc-session=([^;]+)/)
|
||||
const match = setCookie.match(/(?:__Host-)?mc-session=([^;]+)/)
|
||||
expect(match).toBeTruthy()
|
||||
const sessionToken = match![1]
|
||||
const sessionCookiePair = match?.[0] || ''
|
||||
|
||||
// Use the session cookie to access /api/auth/me
|
||||
// Use the same cookie name/value returned by login
|
||||
const meRes = await request.get('/api/auth/me', {
|
||||
headers: { 'cookie': `mc-session=${sessionToken}`, 'x-forwarded-for': '10.88.88.2' }
|
||||
headers: { 'cookie': sessionCookiePair, 'x-forwarded-for': '10.88.88.2' }
|
||||
})
|
||||
expect(meRes.status()).toBe(200)
|
||||
|
||||
const body = await meRes.json()
|
||||
expect(body.user?.username).toBe(TEST_USER)
|
||||
expect(typeof body.user?.workspace_id).toBe('number')
|
||||
|
|
|
|||
Loading…
Reference in New Issue