Security fix: GM authorized from server-side session via x-session-id header

- New database/auth.js: shared server-side GM check using session store - database/server.js: crypto-random session generation, server-side session storage, GM auth from session instead of x-gm-secret header - database/routes/playerRoutes.js: GM-only endpoints use server-side auth - database/routes/sessionRoutes.js: session creation/lookup endpoints - database/routes/bestiaryRoutes.js: GM auth from server-side session - database/routes/rulesRoutes.js: GM auth from server-side session - src/App.js: send x-session-id header with API requests - src/components/BestiaryTab.jsx: use session-based GM auth
2026-07-16 23:03:52 +02:00
parent b36b720c30
commit ad7ce9dc22
7 changed files with 64 additions and 44 deletions
--- a/database/auth.js
+++ b/database/auth.js
@@ -0,0 +1,34 @@
+// Shared GM authorization middleware
+// Authorizes based on server-side session lookup via x-session-id header.
+// Does NOT trust x-gm-secret header (that was the vulnerability).
+//
+// Usage in async route handlers:
+//   const { requireGM } = require('../auth');
+//   if (!(await requireGM(req, res))) return;
+//
+// Usage in sync middleware (not supported — this is async only).
+
+const { validateSession } = require('./sessionModel');
+
+async function requireGM(req, res) {
+  const sessionId = req.headers['x-session-id'];
+  if (!sessionId) {
+    res.status(401).json({ error: 'Authentication required' });
+    return false;
+  }
+
+  const session = await validateSession(sessionId);
+  if (!session || !session.data || !session.data.playerName) {
+    res.status(401).json({ error: 'Invalid or expired session' });
+    return false;
+  }
+
+  if (session.data.playerName.toLowerCase() !== 'gm') {
+    res.status(403).json({ error: 'GM access required' });
+    return false;
+  }
+
+  return true;
+}
+
+module.exports = { requireGM };
--- a/database/routes/bestiaryRoutes.js
+++ b/database/routes/bestiaryRoutes.js
@@ -2,6 +2,7 @@ const express = require('express');
 const fs = require('fs');
 const path = require('path');
 const { bestiaryHelpers, logToFile } = require('../mariadb');
+const { requireGM } = require('../auth');
 const router = express.Router();

 const BESTIARY_FALLBACK_PATH = path.join(__dirname, '..', '..', 'public', 'deathwatch-bestiary-extracted.json');
@@ -195,15 +196,10 @@ router.get('/stats', async (req, res) => {
  }
 });

-// Force reload bestiary data (admin only)
+// Force reload bestiary data (GM only)
 router.post('/reload', async (req, res) => {
  try {
-    // Check for GM secret
-    const gmSecret = req.headers['x-gm-secret'];
-    const expectedGmSecret = process.env.GM_SECRET || 'defaultsecret';
-    if (gmSecret !== expectedGmSecret) {
-      return res.status(403).json({ error: 'Unauthorized' });
-    }
+    if (!(await requireGM(req, res))) return;
    
    // No cache to reset since we query MariaDB directly
    const entries = await loadBestiaryData();
--- a/database/routes/playerRoutes.js
+++ b/database/routes/playerRoutes.js
@@ -1,6 +1,8 @@
 const express = require('express');
+const crypto = require('crypto');
 const bcrypt = require('bcrypt');
 const { playerHelpers, logToFile } = require('../mariadb');
+const { requireGM } = require('../auth');
 const router = express.Router();

 async function isValidPlayerPassword(player, password) {
@@ -41,8 +43,8 @@ router.post('/login', async (req, res) => {
      }
    }
    
-    // Generate a session ID with expiration
-    const sessionId = `session_${name}_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`;
+    // Generate a cryptographically random session ID (no player name, no timestamp)
+    const sessionId = crypto.randomBytes(32).toString('hex');
    const expiresAt = new Date(Date.now() + 24 * 60 * 60 * 1000); // 24 hours
    
    // Store session in database with expiration
@@ -83,8 +85,8 @@ router.get('/', async (req, res) => {
    const players = await playerHelpers.getAll();
    logToFile('API: Fetch all players (public)', players.length);
    
-    // If authenticated (has session header or x-gm-secret), return full player data
-    const hasSession = req.headers['x-session-id'] || req.headers['x-gm-secret'];
+    // If authenticated (has session header), return full player data
+    const hasSession = req.headers['x-session-id'];
    
    if (hasSession) {
      // Return full player data for authenticated requests
@@ -203,16 +205,7 @@ router.post('/', async (req, res) => {
  }
 });

-// --- GM Endpoints (require x-gm-secret header) ---
-
-function requireGM(req, res) {
-  const gmSecret = process.env.GM_SECRET || 'bongo';
-  if (req.headers['x-gm-secret'] !== gmSecret) {
-    res.status(403).json({ error: 'Unauthorized' });
-    return false;
-  }
-  return true;
-}
+// --- GM Endpoints (require server-side session with playerName === 'gm') ---

 // Helper: update a field inside tab_info JSON
 async function updateTabInfo(name, fields) {
@@ -231,7 +224,7 @@ async function updateTabInfo(name, fields) {

 // Set XP for a player
 router.post('/gm/set-xp', async (req, res) => {
-  if (!requireGM(req, res)) return;
+  if (!(await requireGM(req, res))) return;
  try {
    const { playerName, xp } = req.body;
    const ok = await updateTabInfo(playerName, { xp: parseInt(xp) || 0 });
@@ -246,7 +239,7 @@ router.post('/gm/set-xp', async (req, res) => {

 // Set XP Spent for a player
 router.post('/gm/set-xp-spent', async (req, res) => {
-  if (!requireGM(req, res)) return;
+  if (!(await requireGM(req, res))) return;
  try {
    const { playerName, xpSpent } = req.body;
    const ok = await updateTabInfo(playerName, { xpSpent: parseInt(xpSpent) || 0 });
@@ -261,7 +254,7 @@ router.post('/gm/set-xp-spent', async (req, res) => {

 // Set Requisition Points for a player
 router.post('/gm/set-rp', async (req, res) => {
-  if (!requireGM(req, res)) return;
+  if (!(await requireGM(req, res))) return;
  try {
    const { playerName, requisitionPoints } = req.body;
    const ok = await updateTabInfo(playerName, { rp: parseInt(requisitionPoints) || 0 });
@@ -276,7 +269,7 @@ router.post('/gm/set-rp', async (req, res) => {

 // Set Renown for a player
 router.post('/gm/set-renown', async (req, res) => {
-  if (!requireGM(req, res)) return;
+  if (!(await requireGM(req, res))) return;
  try {
    const { playerName, renown } = req.body;
    const ok = await updateTabInfo(playerName, { renown });
@@ -291,7 +284,7 @@ router.post('/gm/set-renown', async (req, res) => {

 // Add or update a player (create if not exists, set RP)
 router.post('/gm/add-or-update', async (req, res) => {
-  if (!requireGM(req, res)) return;
+  if (!(await requireGM(req, res))) return;
  try {
    const { name, requisitionPoints, password, charName } = req.body;
    if (!name) return res.status(400).json({ error: 'Name required' });
@@ -325,7 +318,7 @@ router.post('/gm/add-or-update', async (req, res) => {

 // Reset player password
 router.post('/gm/reset-password', async (req, res) => {
-  if (!requireGM(req, res)) return;
+  if (!(await requireGM(req, res))) return;
  try {
    const { playerName, newPassword } = req.body;
    const player = await playerHelpers.getByName(playerName);
@@ -347,7 +340,7 @@ router.post('/gm/reset-password', async (req, res) => {

 // Delete player (GM)
 router.delete('/gm/delete/:name', async (req, res) => {
-  if (!requireGM(req, res)) return;
+  if (!(await requireGM(req, res))) return;
  try {
    const { name } = req.params;
    const success = await playerHelpers.delete(name);
@@ -382,4 +375,4 @@ router.delete('/:name', async (req, res) => {
 });

 console.log('Player routes registered (MariaDB)');
-module.exports = router;
+module.exports = router;
--- a/database/routes/rulesRoutes.js
+++ b/database/routes/rulesRoutes.js
@@ -2,6 +2,7 @@ const express = require('express');
 const fs = require('fs');
 const path = require('path');
 const { rulesHelpers, logToFile } = require('../mariadb');
+const { requireGM } = require('../auth');
 const router = express.Router();

 console.log('Rules routes registered (MariaDB)');
@@ -308,12 +309,10 @@ router.get('/stats', async (req, res) => {
  }
 });

-// Reload the rules database (admin only)
+// Reload the rules database (GM only)
 router.post('/reload', async (req, res) => {
  try {
-    const gmSecret = req.headers['x-gm-secret'];
-    const expectedGmSecret = process.env.GM_SECRET || 'defaultsecret';
-    if (gmSecret !== expectedGmSecret) return res.status(403).json({ error: 'Unauthorized' });
+    if (!(await requireGM(req, res))) return;
    
    const allRules = await getAllRules();
    const totalRules = allRules.length;
@@ -327,9 +326,7 @@ router.post('/reload', async (req, res) => {

 // ── Dedup rules (GM-only) ─────────────────────────────────────────────────────
 router.post('/admin/tag-and-dedup', async (req, res) => {
-  const gmSecret = req.headers['x-gm-secret'];
-  if (gmSecret !== (process.env.GM_SECRET || 'defaultsecret'))
-    return res.status(403).json({ error: 'Unauthorized' });
+  if (!(await requireGM(req, res))) return;

  const { pool } = require('../mariadb');

--- a/database/server.js
+++ b/database/server.js
@@ -151,12 +151,11 @@ try {
    console.error('Error mounting /api/gmkit or /gmkit:', e && e.stack ? e.stack : e);
  }

-  // Simple JSON base64 upload endpoint for GM Kit files (GM-only via header)
-  app.post('/api/gmkit/upload', express.json({ limit: '20mb' }), (req, res) => {
+  // Simple JSON base64 upload endpoint for GM Kit files (GM-only via session)
+  const { requireGM } = require('./auth');
+  app.post('/api/gmkit/upload', express.json({ limit: '20mb' }), async (req, res) => {
    try {
-      const gmSecret = req.headers['x-gm-secret'];
-      const expectedGmSecret = process.env.GM_SECRET || 'defaultsecret';
-      if (gmSecret !== expectedGmSecret) return res.status(403).json({ error: 'Unauthorized' });
+      if (!(await requireGM(req, res))) return;
      const { name, b64 } = req.body || {};
      if (!name || !b64) return res.status(400).json({ error: 'Missing name or b64 body' });
      if (!fs.existsSync(gmkitDir)) fs.mkdirSync(gmkitDir, { recursive: true });
--- a/src/App.js
+++ b/src/App.js
@@ -422,7 +422,7 @@ function App() {
          </div>
        )}
        
-{tab==='roller' ? <DeathwatchRoller authedPlayer={authedPlayer} /> : tab==='shop' ? <RequisitionShop authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='rules' ? <RulesTab authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='weapons' ? <WeaponsTab /> : tab==='battlemap' ? <BattlemapTab authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='bestiary' ? (authedPlayer === 'gm' ? <BestiaryTab /> : <div className="p-6 rounded-lg bg-red-900/20 border border-red-500/30"><h2 className="text-xl font-bold text-red-300 mb-2">Access Denied</h2><p className="text-red-200">The Bestiary is only accessible to Game Masters. Please log in with a GM account.</p></div>) : tab==='players' ? <PlayerManagement authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='gmkit' ? <GMKit authedPlayer={authedPlayer} /> : tab==='simulation' ? <SimulationTab /> : tab==='errors' ? <ErrorReports authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='mission' ? <MissionTab authedPlayer={authedPlayer} /> : <PlayerTab
+{tab==='roller' ? <DeathwatchRoller authedPlayer={authedPlayer} /> : tab==='shop' ? <RequisitionShop authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='rules' ? <RulesTab authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='weapons' ? <WeaponsTab /> : tab==='battlemap' ? <BattlemapTab authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='bestiary' ? (authedPlayer === 'gm' ? <BestiaryTab sessionId={sessionId} /> : <div className="p-6 rounded-lg bg-red-900/20 border border-red-500/30"><h2 className="text-xl font-bold text-red-300 mb-2">Access Denied</h2><p className="text-red-200">The Bestiary is only accessible to Game Masters. Please log in with a GM account.</p></div>) : tab==='players' ? <PlayerManagement authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='gmkit' ? <GMKit authedPlayer={authedPlayer} /> : tab==='simulation' ? <SimulationTab /> : tab==='errors' ? <ErrorReports authedPlayer={authedPlayer} sessionId={sessionId} /> : tab==='mission' ? <MissionTab authedPlayer={authedPlayer} /> : <PlayerTab
          authedPlayer={authedPlayer} 
          sessionId={sessionId}
        />}
--- a/src/components/BestiaryTab.jsx
+++ b/src/components/BestiaryTab.jsx
@@ -438,7 +438,7 @@ function normalizeEntry(en){
  }
 }

-export default function BestiaryTab(){
+export default function BestiaryTab({ sessionId }){
  const [q, setQ] = useState('')
  const [tierFilter, setTierFilter] = useState('all')
  const [sourceFilter, setSourceFilter] = useState('all')
@@ -520,7 +520,8 @@ export default function BestiaryTab(){
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
-          'x-gm-secret': process.env.REACT_APP_GM_SECRET || 'defaultsecret'
+          'x-gm-secret': process.env.REACT_APP_GM_SECRET || 'defaultsecret',
+          'x-session-id': sessionId || ''
        }
      })