fix(auth): mark auth cookies Secure per connection, not per NODE_ENV
Cookies were Secure whenever NODE_ENV=production, so browsers discarded them on any deployment served over plain HTTP and every login bounced back to the login page. A fixed flag cannot serve a LAN over HTTP and the internet over HTTPS from the same instance. Derive the flag from req.secure, which honours X-Forwarded-Proto from a trusted proxy, and add COOKIE_SECURE=auto|true|false to override it.
This commit is contained in:
@@ -70,6 +70,12 @@ TZ=UTC
|
||||
# DOMAIN=http://192.168.0.118,https://flashpoint.example.com
|
||||
DOMAIN=http://localhost:80
|
||||
|
||||
# Secure flag on auth cookies: auto (default), true, or false. `auto` marks them
|
||||
# Secure only on requests that arrived over TLS, so plain-HTTP LAN access and
|
||||
# HTTPS through a proxy both work. Force `true` if your proxy terminates TLS
|
||||
# without sending X-Forwarded-Proto.
|
||||
# COOKIE_SECURE=auto
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Database Performance Optimization (for network storage)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -30,6 +30,16 @@ function getJwtSecret(): string {
|
||||
return devSecret;
|
||||
}
|
||||
|
||||
function parseCookieSecure(value: string | undefined): boolean | 'auto' {
|
||||
const normalized = value?.trim().toLowerCase();
|
||||
if (normalized === 'true') return true;
|
||||
if (normalized === 'false') return false;
|
||||
if (normalized !== undefined && normalized !== '' && normalized !== 'auto') {
|
||||
console.warn(`⚠️ Ignoring invalid COOKIE_SECURE="${value}" (use true, false, or auto).`);
|
||||
}
|
||||
return 'auto';
|
||||
}
|
||||
|
||||
const getFlashpointPath = (): string => {
|
||||
if (process.env.FLASHPOINT_PATH) {
|
||||
return process.env.FLASHPOINT_PATH;
|
||||
@@ -88,6 +98,11 @@ export const config = {
|
||||
flashpointPlaylistsPath: `${flashpointPath}/Data/Playlists`,
|
||||
flashpointGamesPath: `${flashpointPath}/Data/Games`,
|
||||
|
||||
// Auth cookie Secure flag. 'auto' (default) marks cookies Secure only on
|
||||
// requests that actually arrived over TLS, so one deployment can serve plain
|
||||
// HTTP on a LAN and HTTPS through a proxy. Force with COOKIE_SECURE=true|false.
|
||||
cookieSecure: parseCookieSecure(process.env.COOKIE_SECURE),
|
||||
|
||||
// Primary origin, used where a single value is required (e.g. share links).
|
||||
domain: process.env.DOMAIN || 'http://localhost:5173',
|
||||
// DOMAIN accepts a comma-separated list so one deployment can be reached over
|
||||
|
||||
@@ -134,8 +134,8 @@ router.post(
|
||||
|
||||
logger.info(`[Auth] Login successful for user: ${credentials.username}`);
|
||||
|
||||
setRefreshTokenCookie(res, result.tokens.refreshToken);
|
||||
setAccessTokenCookie(res, result.tokens.accessToken);
|
||||
setRefreshTokenCookie(req, res, result.tokens.refreshToken);
|
||||
setAccessTokenCookie(req, res, result.tokens.accessToken);
|
||||
|
||||
res.json({
|
||||
user: result.user,
|
||||
@@ -213,8 +213,8 @@ router.post(
|
||||
|
||||
logger.info(`[Auth] Registration successful for user: ${data.username}`);
|
||||
|
||||
setRefreshTokenCookie(res, result.tokens.refreshToken);
|
||||
setAccessTokenCookie(res, result.tokens.accessToken);
|
||||
setRefreshTokenCookie(req, res, result.tokens.refreshToken);
|
||||
setAccessTokenCookie(req, res, result.tokens.accessToken);
|
||||
|
||||
res.status(201).json({
|
||||
user: result.user,
|
||||
@@ -256,8 +256,8 @@ router.post(
|
||||
await authService.logout(refreshToken);
|
||||
}
|
||||
|
||||
clearRefreshTokenCookie(res);
|
||||
clearAccessTokenCookie(res);
|
||||
clearRefreshTokenCookie(req, res);
|
||||
clearAccessTokenCookie(req, res);
|
||||
|
||||
res.json({ success: true, message: 'Logged out successfully' });
|
||||
})
|
||||
@@ -276,8 +276,8 @@ router.post(
|
||||
const tokens = await authService.refreshToken(refreshToken);
|
||||
|
||||
// Token rotation: both tokens are replaced on every refresh
|
||||
setRefreshTokenCookie(res, tokens.refreshToken);
|
||||
setAccessTokenCookie(res, tokens.accessToken);
|
||||
setRefreshTokenCookie(req, res, tokens.refreshToken);
|
||||
setAccessTokenCookie(req, res, tokens.accessToken);
|
||||
|
||||
res.json({
|
||||
expiresIn: tokens.expiresIn,
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
import { describe, it, expect, afterEach } from 'vitest';
|
||||
import express from 'express';
|
||||
import type { AddressInfo } from 'net';
|
||||
import type { Server } from 'http';
|
||||
import { resolveSecureFlag, setAccessTokenCookie, setRefreshTokenCookie } from './cookies';
|
||||
|
||||
describe('resolveSecureFlag', () => {
|
||||
describe('auto (default)', () => {
|
||||
it('marks cookies Secure on an HTTPS request', () => {
|
||||
expect(resolveSecureFlag(true, 'auto')).toBe(true);
|
||||
});
|
||||
|
||||
it('omits Secure on a plain HTTP request, so the browser keeps the cookie', () => {
|
||||
expect(resolveSecureFlag(false, 'auto')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('explicit override', () => {
|
||||
it('forces Secure even when the request looks plain, for a proxy that hides TLS', () => {
|
||||
expect(resolveSecureFlag(false, true)).toBe(true);
|
||||
});
|
||||
|
||||
it('forces Secure off even on an HTTPS request', () => {
|
||||
expect(resolveSecureFlag(true, false)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('auth cookies over a real request', () => {
|
||||
let server: Server | undefined;
|
||||
|
||||
afterEach(async () => {
|
||||
if (server) {
|
||||
await new Promise<void>((resolve) => server?.close(() => resolve()));
|
||||
server = undefined;
|
||||
}
|
||||
});
|
||||
|
||||
async function setCookiesVia(trustProxy: boolean, headers: Record<string, string> = {}) {
|
||||
const app = express();
|
||||
if (trustProxy) {
|
||||
app.set('trust proxy', 1);
|
||||
}
|
||||
app.get('/login', (req, res) => {
|
||||
setAccessTokenCookie(req, res, 'access-token');
|
||||
setRefreshTokenCookie(req, res, 'refresh-token');
|
||||
res.status(204).end();
|
||||
});
|
||||
|
||||
server = app.listen(0);
|
||||
await new Promise<void>((resolve) => server?.once('listening', () => resolve()));
|
||||
const { port } = server.address() as AddressInfo;
|
||||
|
||||
const response = await fetch(`http://127.0.0.1:${port}/login`, { headers });
|
||||
return response.headers.getSetCookie();
|
||||
}
|
||||
|
||||
it('omits Secure over plain HTTP so the browser stores the cookies', async () => {
|
||||
const cookies = await setCookiesVia(false);
|
||||
|
||||
expect(cookies).toHaveLength(2);
|
||||
expect(cookies.every((cookie) => !cookie.includes('Secure'))).toBe(true);
|
||||
expect(cookies.every((cookie) => cookie.includes('HttpOnly'))).toBe(true);
|
||||
});
|
||||
|
||||
it('adds Secure when a trusted proxy reports the request arrived over HTTPS', async () => {
|
||||
const cookies = await setCookiesVia(true, { 'X-Forwarded-Proto': 'https' });
|
||||
|
||||
expect(cookies).toHaveLength(2);
|
||||
expect(cookies.every((cookie) => cookie.includes('Secure'))).toBe(true);
|
||||
});
|
||||
|
||||
it('ignores a forwarded protocol claim when the proxy is not trusted', async () => {
|
||||
const cookies = await setCookiesVia(false, { 'X-Forwarded-Proto': 'https' });
|
||||
|
||||
expect(cookies.every((cookie) => !cookie.includes('Secure'))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -1,28 +1,51 @@
|
||||
import { CookieOptions, Response } from 'express';
|
||||
import { CookieOptions, Request, Response } from 'express';
|
||||
import { config } from '../config';
|
||||
|
||||
const REFRESH_COOKIE_NAME = 'fp_refresh';
|
||||
const ACCESS_COOKIE_NAME = 'fp_access';
|
||||
const isProduction = config.nodeEnv === 'production';
|
||||
|
||||
function getRefreshCookieOptions(): CookieOptions {
|
||||
/**
|
||||
* Whether to mark auth cookies Secure.
|
||||
*
|
||||
* Derived per request rather than from NODE_ENV: a browser discards a Secure
|
||||
* cookie delivered over plain HTTP, so a fixed `true` breaks every LAN
|
||||
* deployment served without TLS, while a fixed `false` would strip the flag from
|
||||
* genuinely HTTPS traffic. One deployment can be reached both ways, so the
|
||||
* connection at hand decides.
|
||||
*
|
||||
* COOKIE_SECURE overrides the detection when an operator knows better —
|
||||
* for example a TLS-terminating proxy that does not send X-Forwarded-Proto.
|
||||
*/
|
||||
export function resolveSecureFlag(
|
||||
isRequestSecure: boolean,
|
||||
override: boolean | 'auto' = config.cookieSecure
|
||||
): boolean {
|
||||
return override === 'auto' ? isRequestSecure : override;
|
||||
}
|
||||
|
||||
function isSecureRequest(req: Request): boolean {
|
||||
// req.secure honours X-Forwarded-Proto because `trust proxy` is enabled.
|
||||
return resolveSecureFlag(req.secure);
|
||||
}
|
||||
|
||||
function getRefreshCookieOptions(req: Request): CookieOptions {
|
||||
return {
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: isSecureRequest(req),
|
||||
sameSite: 'lax',
|
||||
path: '/api/auth',
|
||||
maxAge: 30 * 24 * 60 * 60 * 1000,
|
||||
};
|
||||
}
|
||||
|
||||
export function setRefreshTokenCookie(res: Response, refreshToken: string): void {
|
||||
res.cookie(REFRESH_COOKIE_NAME, refreshToken, getRefreshCookieOptions());
|
||||
export function setRefreshTokenCookie(req: Request, res: Response, refreshToken: string): void {
|
||||
res.cookie(REFRESH_COOKIE_NAME, refreshToken, getRefreshCookieOptions(req));
|
||||
}
|
||||
|
||||
export function clearRefreshTokenCookie(res: Response): void {
|
||||
export function clearRefreshTokenCookie(req: Request, res: Response): void {
|
||||
res.clearCookie(REFRESH_COOKIE_NAME, {
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: isSecureRequest(req),
|
||||
sameSite: 'lax',
|
||||
path: '/api/auth',
|
||||
});
|
||||
@@ -32,24 +55,24 @@ export function getRefreshTokenFromCookie(cookies: Record<string, string>): stri
|
||||
return cookies?.[REFRESH_COOKIE_NAME];
|
||||
}
|
||||
|
||||
function getAccessCookieOptions(): CookieOptions {
|
||||
function getAccessCookieOptions(req: Request): CookieOptions {
|
||||
return {
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: isSecureRequest(req),
|
||||
sameSite: 'lax',
|
||||
path: '/api',
|
||||
maxAge: 60 * 60 * 1000,
|
||||
};
|
||||
}
|
||||
|
||||
export function setAccessTokenCookie(res: Response, accessToken: string): void {
|
||||
res.cookie(ACCESS_COOKIE_NAME, accessToken, getAccessCookieOptions());
|
||||
export function setAccessTokenCookie(req: Request, res: Response, accessToken: string): void {
|
||||
res.cookie(ACCESS_COOKIE_NAME, accessToken, getAccessCookieOptions(req));
|
||||
}
|
||||
|
||||
export function clearAccessTokenCookie(res: Response): void {
|
||||
export function clearAccessTokenCookie(req: Request, res: Response): void {
|
||||
res.clearCookie(ACCESS_COOKIE_NAME, {
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: isSecureRequest(req),
|
||||
sameSite: 'lax',
|
||||
path: '/api',
|
||||
});
|
||||
|
||||
@@ -30,6 +30,7 @@ Location: `backend/.env` (local development)
|
||||
| `JWT_EXPIRES_IN` | 1h | Access token expiration (15m, 1h, 7d, etc.) |
|
||||
| `BCRYPT_SALT_ROUNDS` | 10 | Password hash cost (higher = more secure but slower) |
|
||||
| `DOMAIN` | http://localhost:5173 | Allowed CORS origin(s), comma-separated (see below) |
|
||||
| `COOKIE_SECURE` | auto | Secure flag on auth cookies: `auto`, `true`, `false` |
|
||||
|
||||
Generate secure JWT secret:
|
||||
|
||||
@@ -37,6 +38,17 @@ Generate secure JWT secret:
|
||||
openssl rand -hex 64
|
||||
```
|
||||
|
||||
**COOKIE_SECURE — auth cookies over HTTP and HTTPS:**
|
||||
|
||||
Browsers discard a cookie marked `Secure` when it arrives over plain HTTP, which
|
||||
logs users straight back out. `auto` therefore decides per request: cookies are
|
||||
marked `Secure` only on connections that actually used TLS, so one deployment can
|
||||
serve a LAN over HTTP and the internet over HTTPS at the same time.
|
||||
|
||||
Detection reads `X-Forwarded-Proto`, which is trusted only from a proxy. Set
|
||||
`COOKIE_SECURE=true` if your TLS-terminating proxy does not send that header, and
|
||||
`false` only if you deliberately want the flag off everywhere.
|
||||
|
||||
**DOMAIN — allowed origins:**
|
||||
|
||||
`DOMAIN` accepts one origin or a comma-separated list, so a single deployment can
|
||||
|
||||
Reference in New Issue
Block a user