fix(security #7,#8,#9): signed cookies, isAdmin tamper-proof, schema validation
#7 isAdmin trusted from unsigned cookie: - isAdmin is derived server-side from Emby Policy at login time - Cookie is now signed (HMAC) when COOKIE_SECRET env var is set; Express rejects tampered signatures (signedCookies returns false) - dashboard.js /user-downloads and /status now use requireAuth middleware (req.user) instead of re-parsing cookie directly #8 cookie-parser used without signing secret: - cookieParser(COOKIE_SECRET) in index.js when env var is set - Hard-fails at startup in production if COOKIE_SECRET unset - Warns in development #9 Cookie JSON parsed without schema validation: - parseSessionCookie() in auth.js and requireAuth.js both validate: id (non-empty string), name (non-empty string), isAdmin (boolean) - Invalid/tampered cookies return null / 401 respectively
This commit is contained in:
@@ -58,7 +58,14 @@ const { startPoller, POLL_INTERVAL, POLLING_ENABLED } = require('./utils/poller'
|
||||
const app = express();
|
||||
const PORT = process.env.PORT || 3001;
|
||||
|
||||
app.use(cookieParser());
|
||||
const cookieSecret = process.env.COOKIE_SECRET;
|
||||
if (!cookieSecret && process.env.NODE_ENV === 'production') {
|
||||
console.error('[Security] COOKIE_SECRET is not set in production — cookies are unsigned and can be tampered with!');
|
||||
process.exit(1);
|
||||
} else if (!cookieSecret) {
|
||||
console.warn('[Security] COOKIE_SECRET is not set — using unsigned cookies (acceptable for development only)');
|
||||
}
|
||||
app.use(cookieParser(cookieSecret || undefined));
|
||||
app.use(express.json());
|
||||
app.use(express.static(path.join(__dirname, '../public')));
|
||||
|
||||
|
||||
@@ -1,13 +1,20 @@
|
||||
function requireAuth(req, res, next) {
|
||||
const userCookie = req.cookies.emby_user;
|
||||
if (!userCookie) {
|
||||
const signed = !!process.env.COOKIE_SECRET;
|
||||
const raw = signed ? req.signedCookies.emby_user : req.cookies.emby_user;
|
||||
if (!raw || raw === false) {
|
||||
return res.status(401).json({ error: 'Not authenticated' });
|
||||
}
|
||||
let u;
|
||||
try {
|
||||
req.user = JSON.parse(userCookie);
|
||||
u = JSON.parse(raw);
|
||||
} catch {
|
||||
return res.status(401).json({ error: 'Invalid session' });
|
||||
}
|
||||
// Schema validation
|
||||
if (typeof u.id !== 'string' || !u.id) return res.status(401).json({ error: 'Invalid session' });
|
||||
if (typeof u.name !== 'string' || !u.name) return res.status(401).json({ error: 'Invalid session' });
|
||||
if (typeof u.isAdmin !== 'boolean') u.isAdmin = !!u.isAdmin;
|
||||
req.user = u;
|
||||
next();
|
||||
}
|
||||
|
||||
|
||||
@@ -40,17 +40,17 @@ router.post('/login', loginLimiter, async (req, res) => {
|
||||
const user = userResponse.data;
|
||||
console.log(`[Auth] Login successful for user: ${user.Name}, isAdmin: ${!!(user.Policy && user.Policy.IsAdministrator)}`);
|
||||
|
||||
// Set authentication cookie
|
||||
// Note: token is intentionally excluded from the cookie — it is not needed client-side
|
||||
// Set authentication cookie.
|
||||
// Note: token is intentionally excluded from the cookie — it is not needed client-side.
|
||||
// Cookie is signed when COOKIE_SECRET is set (recommended in production).
|
||||
const isAdmin = !!(user.Policy && user.Policy.IsAdministrator);
|
||||
res.cookie('emby_user', JSON.stringify({
|
||||
id: user.Id,
|
||||
name: user.Name,
|
||||
isAdmin: isAdmin
|
||||
}), {
|
||||
const cookiePayload = JSON.stringify({ id: user.Id, name: user.Name, isAdmin });
|
||||
const signed = !!process.env.COOKIE_SECRET;
|
||||
res.cookie('emby_user', cookiePayload, {
|
||||
httpOnly: true,
|
||||
secure: process.env.NODE_ENV === 'production',
|
||||
sameSite: 'strict',
|
||||
signed,
|
||||
maxAge: 24 * 60 * 60 * 1000 // 24 hours
|
||||
});
|
||||
|
||||
@@ -71,28 +71,30 @@ router.post('/login', loginLimiter, async (req, res) => {
|
||||
}
|
||||
});
|
||||
|
||||
function parseSessionCookie(req) {
|
||||
const signed = !!process.env.COOKIE_SECRET;
|
||||
const raw = signed ? req.signedCookies.emby_user : req.cookies.emby_user;
|
||||
if (!raw || raw === false) return null; // false = tampered signed cookie
|
||||
try {
|
||||
const u = JSON.parse(raw);
|
||||
// Schema validation: require id (string), name (string), isAdmin (boolean)
|
||||
if (typeof u.id !== 'string' || !u.id) return null;
|
||||
if (typeof u.name !== 'string' || !u.name) return null;
|
||||
if (typeof u.isAdmin !== 'boolean') u.isAdmin = !!u.isAdmin;
|
||||
return u;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// Get current authenticated user
|
||||
router.get('/me', (req, res) => {
|
||||
try {
|
||||
const userCookie = req.cookies.emby_user;
|
||||
|
||||
if (!userCookie) {
|
||||
return res.json({ authenticated: false });
|
||||
}
|
||||
|
||||
const user = JSON.parse(userCookie);
|
||||
res.json({
|
||||
authenticated: true,
|
||||
user: {
|
||||
id: user.id,
|
||||
name: user.name,
|
||||
isAdmin: !!user.isAdmin
|
||||
}
|
||||
});
|
||||
} catch (error) {
|
||||
console.error(`[Auth] Error getting current user:`, error.message);
|
||||
res.json({ authenticated: false });
|
||||
}
|
||||
const user = parseSessionCookie(req);
|
||||
if (!user) return res.json({ authenticated: false });
|
||||
res.json({
|
||||
authenticated: true,
|
||||
user: { id: user.id, name: user.name, isAdmin: user.isAdmin }
|
||||
});
|
||||
});
|
||||
|
||||
// Logout
|
||||
@@ -100,7 +102,8 @@ router.post('/logout', (req, res) => {
|
||||
res.clearCookie('emby_user', {
|
||||
httpOnly: true,
|
||||
secure: process.env.NODE_ENV === 'production',
|
||||
sameSite: 'strict'
|
||||
sameSite: 'strict',
|
||||
signed: !!process.env.COOKIE_SECRET
|
||||
});
|
||||
res.json({ success: true });
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
const express = require('express');
|
||||
const router = express.Router();
|
||||
const requireAuth = require('../middleware/requireAuth');
|
||||
|
||||
const axios = require('axios');
|
||||
const { mapTorrentToDownload } = require('../utils/qbittorrent');
|
||||
@@ -143,15 +144,9 @@ function getActiveClients() {
|
||||
}
|
||||
|
||||
// Get user downloads for authenticated user
|
||||
router.get('/user-downloads', async (req, res) => {
|
||||
router.get('/user-downloads', requireAuth, async (req, res) => {
|
||||
try {
|
||||
// Get authenticated user from cookie
|
||||
const userCookie = req.cookies.emby_user;
|
||||
if (!userCookie) {
|
||||
return res.status(401).json({ error: 'Not authenticated' });
|
||||
}
|
||||
|
||||
const user = JSON.parse(userCookie);
|
||||
const user = req.user;
|
||||
const username = user.name.toLowerCase();
|
||||
const usernameSanitized = sanitizeTagLabel(user.name);
|
||||
const isAdmin = !!user.isAdmin;
|
||||
@@ -622,7 +617,7 @@ router.get('/user-downloads', async (req, res) => {
|
||||
});
|
||||
|
||||
// Get all users with their download counts
|
||||
router.get('/user-summary', async (req, res) => {
|
||||
router.get('/user-summary', requireAuth, async (req, res) => {
|
||||
try {
|
||||
const sonarrInstances = getSonarrInstances();
|
||||
const radarrInstances = getRadarrInstances();
|
||||
@@ -696,13 +691,9 @@ router.get('/user-summary', async (req, res) => {
|
||||
});
|
||||
|
||||
// Admin-only status page with cache stats
|
||||
router.get('/status', (req, res) => {
|
||||
router.get('/status', requireAuth, (req, res) => {
|
||||
try {
|
||||
const userCookie = req.cookies.emby_user;
|
||||
if (!userCookie) {
|
||||
return res.status(401).json({ error: 'Not authenticated' });
|
||||
}
|
||||
const user = JSON.parse(userCookie);
|
||||
const user = req.user;
|
||||
if (!user.isAdmin) {
|
||||
return res.status(403).json({ error: 'Admin access required' });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user