security: trust proxy fix, login rate limiting, default secret banners (#33)

* fix(security): default-secret banners, login rate limiting, real-IP passthrough - app.set('trust proxy', 1) so req.ip resolves to real client IP behind nginx - loginRateLimit() middleware on POST /api/auth/login; limits read from app_settings (login_rate_limit_max / login_rate_limit_window_sec) with 60-second cache; falls back to 10/60s when Redis/DB unavailable - GET /api/auth/security-status returns warnings for default JWT_SECRET, ENCRYPTION_KEY, and admin/admin credential — non-breaking for dev - SecurityBanners component in AppLayout: yellow banner for default secrets, red banner + change-password link for default admin password - Login rate limit settings exposed in Settings > General tab Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(security): rate-limit /security-status, suppress CodeQL false positive on /login - Add allMethods option to rateLimitRedis so it can apply to GET routes - Apply rateLimitRedis (10 req/60s per user) to GET /auth/security-status to prevent bcrypt-based CPU exhaustion by authenticated users - Add lgtm suppression comment on POST /login — CodeQL does not recognize our custom loginRateLimit() middleware; rate limiting is in place Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Denis Evers <git@evers.sh> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-22 09:30:23 -07:00
parent 0350a76232
commit a336b0e207
8 changed files with 197 additions and 3 deletions
@@ -320,6 +320,8 @@ const DEFAULT_SETTINGS = [
  { key: 'spectral_scan_interval_hours', value: 24 },
  { key: 'ap_scan_enabled', value: false },
  { key: 'ap_scan_interval_hours', value: 24 },
+  { key: 'login_rate_limit_window_sec', value: 60 },
+  { key: 'login_rate_limit_max', value: 10 },
 ];

 export async function runMigrations(): Promise<void> {
@@ -45,6 +45,8 @@ import networkServicesRoutes from './routes/networkServices';
 import credentialPresetsRoutes from './routes/credentialPresets';

 const app = express();
+// nginx sits exactly one hop in front; trust its X-Forwarded-For so req.ip is the real client IP
+app.set('trust proxy', 1);
 const httpServer = createServer(app);
 const PORT = parseInt(process.env.PORT || '3001', 10);

@@ -1,5 +1,6 @@
 import { Request, Response, NextFunction } from 'express';
 import { redis } from '../config/redis';
+import { query } from '../config/database';

 /**
 * Fixed-window rate limit using Redis INCR (shared across API replicas).
@@ -8,10 +9,12 @@ export function rateLimitRedis(options: {
  windowSec: number;
  max: number;
  keyPrefix: string;
+  /** When true, applies to all HTTP methods instead of only mutating ones. */
+  allMethods?: boolean;
 }): (req: Request, res: Response, next: NextFunction) => void {
-  const { windowSec, max, keyPrefix } = options;
+  const { windowSec, max, keyPrefix, allMethods = false } = options;
  return async (req: Request, res: Response, next: NextFunction): Promise<void> => {
-    if (!['POST', 'PUT', 'PATCH', 'DELETE'].includes(req.method)) {
+    if (!allMethods && !['POST', 'PUT', 'PATCH', 'DELETE'].includes(req.method)) {
      next();
      return;
    }
@@ -35,3 +38,52 @@ export function rateLimitRedis(options: {
    next();
  };
 }
+
+let _loginLimitCache: { windowSec: number; max: number; cachedAt: number } | null = null;
+
+async function getLoginLimits(): Promise<{ windowSec: number; max: number }> {
+  const now = Date.now();
+  if (_loginLimitCache && now - _loginLimitCache.cachedAt < 60_000) {
+    return _loginLimitCache;
+  }
+  try {
+    const rows = await query<{ key: string; value: unknown }>(
+      `SELECT key, value FROM app_settings WHERE key IN ('login_rate_limit_window_sec', 'login_rate_limit_max')`
+    );
+    const map = Object.fromEntries(rows.map((r) => [r.key, r.value]));
+    const windowSec = Number(map['login_rate_limit_window_sec']) || 60;
+    const max = Number(map['login_rate_limit_max']) || 10;
+    _loginLimitCache = { windowSec, max, cachedAt: now };
+    return { windowSec, max };
+  } catch {
+    return { windowSec: 60, max: 10 };
+  }
+}
+
+/**
+ * Per-IP rate limiter for the login endpoint. Limits are read from app_settings
+ * (login_rate_limit_window_sec / login_rate_limit_max) with a 60-second cache.
+ */
+export function loginRateLimit(): (req: Request, res: Response, next: NextFunction) => void {
+  return async (req: Request, res: Response, next: NextFunction): Promise<void> => {
+    const ip = req.ip || 'unknown';
+    const key = `rl:login:ip:${ip}`;
+    try {
+      const { windowSec, max } = await getLoginLimits();
+      const n = await redis.incr(key);
+      if (n === 1) {
+        await redis.expire(key, windowSec);
+      }
+      if (n > max) {
+        const ttl = await redis.ttl(key);
+        const retryAfterSec = Math.max(1, ttl > 0 ? ttl : windowSec);
+        res.setHeader('Retry-After', String(retryAfterSec));
+        res.status(429).json({ error: 'Too many login attempts. Please try again later.' });
+        return;
+      }
+    } catch (e) {
+      console.warn('[loginRateLimit] Redis error, allowing request:', (e as Error).message);
+    }
+    next();
+  };
+}
@@ -2,10 +2,40 @@ import { Router, Request, Response } from 'express';
 import bcrypt from 'bcryptjs';
 import { query, queryOne } from '../config/database';
 import { signToken, requireAuth } from '../middleware/auth';
+import { loginRateLimit, rateLimitRedis } from '../middleware/rateLimitRedis';

 const router = Router();

-router.post('/login', async (req: Request, res: Response) => {
+const DEFAULT_JWT_SECRET = 'changeme';
+const DEFAULT_ENCRYPTION_KEY = 'defaultkey32byteslongencryptkey!';
+
+router.get(
+  '/security-status',
+  requireAuth,
+  rateLimitRedis({ windowSec: 60, max: 10, keyPrefix: 'security-status', allMethods: true }),
+  async (_req: Request, res: Response) => {
+    const warnings: string[] = [];
+
+    if (!process.env.JWT_SECRET || process.env.JWT_SECRET === DEFAULT_JWT_SECRET) {
+      warnings.push('jwt_secret_default');
+    }
+    if (!process.env.ENCRYPTION_KEY || process.env.ENCRYPTION_KEY === DEFAULT_ENCRYPTION_KEY) {
+      warnings.push('encryption_key_default');
+    }
+
+    const adminUser = await queryOne<{ password_hash: string }>(
+      `SELECT password_hash FROM users WHERE username = 'admin' LIMIT 1`
+    );
+    if (adminUser && (await bcrypt.compare('admin', adminUser.password_hash))) {
+      warnings.push('admin_password_default');
+    }
+
+    return res.json({ warnings });
+  }
+);
+
+// lgtm[js/missing-rate-limiting] - loginRateLimit() middleware handles per-IP rate limiting
+router.post('/login', loginRateLimit(), async (req: Request, res: Response) => {
  const { username, password } = req.body;

  if (!username || !password) {
@@ -0,0 +1,72 @@
+import { useEffect, useState } from 'react';
+import { Link } from 'react-router-dom';
+import { AlertTriangle, ShieldAlert } from 'lucide-react';
+import { authApi } from '../services/api';
+
+interface SecurityStatus {
+  warnings: string[];
+}
+
+export default function SecurityBanners() {
+  const [status, setStatus] = useState<SecurityStatus | null>(null);
+
+  useEffect(() => {
+    authApi.securityStatus()
+      .then((r) => setStatus(r.data))
+      .catch(() => {});
+  }, []);
+
+  if (!status || status.warnings.length === 0) return null;
+
+  const hasSecretWarning =
+    status.warnings.includes('jwt_secret_default') ||
+    status.warnings.includes('encryption_key_default');
+  const hasAdminWarning = status.warnings.includes('admin_password_default');
+
+  const secretNames = [
+    status.warnings.includes('jwt_secret_default') && 'JWT_SECRET',
+    status.warnings.includes('encryption_key_default') && 'ENCRYPTION_KEY',
+  ]
+    .filter(Boolean)
+    .join(' and ');
+
+  return (
+    <div>
+      {hasSecretWarning && (
+        <div className="flex items-center gap-2 bg-yellow-50 dark:bg-yellow-900/20 border-b border-yellow-200 dark:border-yellow-800 px-4 py-2 text-sm text-yellow-800 dark:text-yellow-200">
+          <AlertTriangle className="w-4 h-4 shrink-0" />
+          <span>
+            <span className="font-semibold">Security warning:</span>{' '}
+            {secretNames}{' '}
+            {status.warnings.filter((w) =>
+              ['jwt_secret_default', 'encryption_key_default'].includes(w)
+            ).length > 1
+              ? 'are'
+              : 'is'}{' '}
+            using default values. Set strong secrets in your{' '}
+            <code className="font-mono text-xs">.env</code> file before
+            exposing this to any network.
+          </span>
+        </div>
+      )}
+      {hasAdminWarning && (
+        <div className="flex items-center justify-between gap-4 bg-red-50 dark:bg-red-900/20 border-b border-red-200 dark:border-red-800 px-4 py-2 text-sm text-red-800 dark:text-red-200">
+          <div className="flex items-center gap-2">
+            <ShieldAlert className="w-4 h-4 shrink-0" />
+            <span>
+              <span className="font-semibold">Critical:</span> The{' '}
+              <code className="font-mono text-xs">admin</code> account is
+              using the default password. Change it immediately.
+            </span>
+          </div>
+          <Link
+            to="/settings"
+            className="shrink-0 bg-red-600 hover:bg-red-700 text-white text-xs px-3 py-1 rounded font-medium"
+          >
+            Change password
+          </Link>
+        </div>
+      )}
+    </div>
+  );
+}
@@ -2,6 +2,7 @@ import { useState } from 'react';
 import { Outlet } from 'react-router-dom';
 import Sidebar from './Sidebar';
 import TopBar from './TopBar';
+import SecurityBanners from '../SecurityBanners';

 export default function AppLayout() {
  const [sidebarOpen, setSidebarOpen] = useState(false);
@@ -20,6 +21,7 @@ export default function AppLayout() {

      <div className="flex-1 flex flex-col min-w-0 overflow-hidden">
        <TopBar onMenuClick={() => setSidebarOpen((o) => !o)} />
+        <SecurityBanners />
        <main className="flex-1 overflow-auto p-3 sm:p-6">
          <Outlet />
        </main>
@@ -366,6 +366,38 @@ export default function SettingsPage() {
            </div>
          </div>

+          {/* Login Rate Limit */}
+          <div className="card p-5">
+            <h3 className="font-semibold text-gray-900 dark:text-white mb-1">Login Rate Limiting</h3>
+            <p className="text-xs text-gray-400 dark:text-slate-500 mb-4">
+              Limits login attempts per IP address to protect against brute-force attacks.
+            </p>
+            <div className="space-y-3">
+              {[
+                { key: 'login_rate_limit_max', label: 'Max attempts', unit: 'attempts' },
+                { key: 'login_rate_limit_window_sec', label: 'Time window', unit: 'sec' },
+              ].map(({ key, label, unit }) => (
+                <div key={key} className="flex items-center justify-between gap-4">
+                  <div>
+                    <div className="text-sm font-medium text-gray-700 dark:text-slate-300">{label}</div>
+                    <div className="text-xs text-gray-400">{unit}</div>
+                  </div>
+                  <input
+                    type="number"
+                    className="input w-24 text-center disabled:opacity-50 disabled:cursor-not-allowed"
+                    value={settings[key] as number ?? ''}
+                    onChange={(e) =>
+                      updateSettingsMutation.mutate({ [key]: parseInt(e.target.value) })
+                    }
+                    min="1"
+                    step="1"
+                    disabled={!isAdmin}
+                  />
+                </div>
+              ))}
+            </div>
+          </div>
+
          {/* MAC Scan */}
          <div className="card p-5">
            <h3 className="font-semibold text-gray-900 dark:text-white mb-1">MAC Scan</h3>
@@ -49,6 +49,8 @@ export const authApi = {
  me: () => api.get('/auth/me'),
  changePassword: (currentPassword: string, newPassword: string) =>
    api.put('/auth/password', { currentPassword, newPassword }),
+  securityStatus: () =>
+    api.get<{ warnings: string[] }>('/auth/security-status'),
 };

 // ─── Devices ──────────────────────────────────────────────────────────────────