Implement per-IP gallery lockouts and UI controls (#42)

backend/src/routes/adminSettings.js

@@ -19,6 +19,7 @@ const {
 } = require('../services/publicSiteService');
 const { sanitizeCss } = require('../utils/cssSanitizer');
 const { clearShareLinkSettingsCache } = require('../services/shareLinkService');
+const { resetSecurityConfigCache } = require('../utils/authSecurity');
 const router = express.Router();
 const { clearMaxFilesPerUploadCache, MAX_ALLOWED_FILES_PER_UPLOAD } = require('../services/uploadSettings');
 
@@ -590,6 +591,8 @@ router.put('/security', adminAuth, async (req, res) => {
         });
     }
 
+    resetSecurityConfigCache();
+
     // Log activity
     await db('activity_logs').insert({
       activity_type: 'security_settings_updated',

backend/src/routes/auth-enhanced-v2.js

@@ -12,13 +12,14 @@ const {
   checkSuspiciousActivity,
   getGenericAuthError
 } = require('../utils/authSecurity');
-const { 
+const {
   validatePasswordInContext,
   getBcryptRounds,
   logPasswordValidationFailure
 } = require('../utils/passwordValidation');
 const { endSession } = require('../middleware/sessionTimeout');
 const logger = require('../utils/logger');
+const { getClientIp } = require('../utils/requestIp');
 const router = express.Router();
 
 // Admin login with enhanced security
@@ -33,7 +34,7 @@ router.post('/admin/login', [
     }
     
     const { username, password, recaptchaToken } = req.body;
-    const ipAddress = req.ip || req.connection.remoteAddress;
+    const ipAddress = getClientIp(req);
     const userAgent = req.headers['user-agent'] || '';
     
     // Check account lockout first
@@ -175,7 +176,7 @@ router.post('/admin/change-password', [
     logger.info('Admin password changed', {
       userId: adminId,
       username: admin.username,
-      ip: req.ip
+      ip: ipAddress
     });
     
     res.json({ 
@@ -229,14 +230,14 @@ router.post('/gallery/verify', [
     }
     
     const { slug, password, recaptchaToken } = req.body;
-    const ipAddress = req.ip || req.connection.remoteAddress;
+    const ipAddress = getClientIp(req);
     const userAgent = req.headers['user-agent'] || '';
     
     const event = await db('events').where({ slug, is_active: formatBoolean(true), is_archived: formatBoolean(false) }).first();
     const requiresPassword = !(event && (event.require_password === false || event.require_password === 0 || event.require_password === '0'));
 
     if (requiresPassword) {
-      const lockoutStatus = await checkAccountLockout(`gallery:${slug}`);
+      const lockoutStatus = await checkAccountLockout(`gallery:${slug}`, ipAddress);
       if (lockoutStatus.isLocked) {
         logger.warn('Gallery access attempt on locked gallery', { slug, ipAddress });
         return res.status(423).json({ 

backend/src/routes/auth-enhanced.js

@@ -23,6 +23,7 @@ const {
   getGalleryTokenFromRequest,
 } = require('../utils/tokenUtils');
 const { getEventShareToken, resolveShareIdentifier } = require('../services/shareLinkService');
+const { getClientIp } = require('../utils/requestIp');
 const router = express.Router();
 
 // Admin login with enhanced security
@@ -37,7 +38,7 @@ router.post('/admin/login', [
     }
     
     const { username, password, recaptchaToken } = req.body;
-    const ipAddress = req.ip || req.connection.remoteAddress;
+    const ipAddress = getClientIp(req);
     const userAgent = req.headers['user-agent'] || '';
     
     // Check account lockout first
@@ -172,7 +173,7 @@ router.post('/gallery/verify', [
     }
     
     const { slug, password, recaptchaToken } = req.body;
-    const ipAddress = req.ip || req.connection.remoteAddress;
+    const ipAddress = getClientIp(req);
     const userAgent = req.headers['user-agent'] || '';
     const event = await db('events')
       .where({ slug, is_active: formatBoolean(true), is_archived: formatBoolean(false) })
@@ -186,7 +187,7 @@ router.post('/gallery/verify', [
     const requiresPassword = !(event.require_password === false || event.require_password === 0 || event.require_password === '0');
 
     if (requiresPassword) {
-      const lockoutStatus = await checkAccountLockout(`gallery:${slug}`);
+      const lockoutStatus = await checkAccountLockout(`gallery:${slug}`, ipAddress);
       if (lockoutStatus.isLocked) {
         logger.warn('Gallery access attempt on locked gallery', { slug, ipAddress });
         return res.status(423).json({ 
@@ -282,7 +283,7 @@ router.post('/gallery/share-login', [
     }
 
     const { slug, token } = req.body;
-    const ipAddress = req.ip || req.connection.remoteAddress;
+    const ipAddress = getClientIp(req);
     const userAgent = req.headers['user-agent'] || '';
 
     let event = await db('events')

backend/src/utils/authSecurity.js

@@ -7,10 +7,140 @@ const { db } = require('../database/db');
 const { formatBoolean } = require('./dbCompat');
 const logger = require('./logger');
 
-// Configuration constants
-const MAX_LOGIN_ATTEMPTS = 5;
-const LOCKOUT_DURATION = 30 * 60 * 1000; // 30 minutes in milliseconds
-const ATTEMPT_WINDOW = 15 * 60 * 1000; // 15 minutes window for counting attempts
+const DEFAULT_SECURITY_CONFIG = Object.freeze({
+  maxAttempts: 5,
+  lockoutDurationMs: 30 * 60 * 1000, // 30 minutes
+  attemptWindowMs: 15 * 60 * 1000 // 15 minutes
+});
+
+const SECURITY_CONFIG_CACHE_MS = 60 * 1000; // 1 minute cache
+let cachedSecurityConfig = { ...DEFAULT_SECURITY_CONFIG };
+let cachedConfigFetchedAt = 0;
+
+function parseStoredValue(rawValue) {
+  if (rawValue === undefined || rawValue === null) {
+    return undefined;
+  }
+
+  if (typeof rawValue !== 'string') {
+    return rawValue;
+  }
+
+  try {
+    return JSON.parse(rawValue);
+  } catch (error) {
+    logger.warn(`Unable to parse stored security setting value "${rawValue}", using raw string.`);
+    return rawValue;
+  }
+}
+
+function normalizePositiveInteger(name, value, fallback, options = {}) {
+  if (value === undefined || value === null || value === '') {
+    return fallback;
+  }
+
+  const numericValue = Number(value);
+
+  if (!Number.isFinite(numericValue)) {
+    logger.warn(`Invalid numeric value for ${name}: ${value}. Falling back to default (${fallback}).`);
+    return fallback;
+  }
+
+  let adjustedValue = Math.floor(numericValue);
+
+  if (options.min !== undefined && adjustedValue < options.min) {
+    logger.warn(`Value for ${name} below minimum (${options.min}). Clamping to minimum.`);
+    adjustedValue = options.min;
+  }
+
+  if (options.max !== undefined && adjustedValue > options.max) {
+    logger.warn(`Value for ${name} exceeds maximum (${options.max}). Clamping to maximum.`);
+    adjustedValue = options.max;
+  }
+
+  if (adjustedValue <= 0) {
+    logger.warn(`Value for ${name} must be positive. Falling back to default (${fallback}).`);
+    return fallback;
+  }
+
+  return adjustedValue;
+}
+
+async function loadSecurityConfigFromSettings() {
+  const rows = await db('app_settings').whereIn('setting_key', [
+    'security_max_login_attempts',
+    'security_lockout_duration_minutes',
+    'security_attempt_window_minutes'
+  ]);
+
+  const config = { ...DEFAULT_SECURITY_CONFIG };
+
+  rows.forEach(row => {
+    const value = parseStoredValue(row.setting_value);
+
+    switch (row.setting_key) {
+      case 'security_max_login_attempts': {
+        config.maxAttempts = normalizePositiveInteger(
+          'security_max_login_attempts',
+          value,
+          DEFAULT_SECURITY_CONFIG.maxAttempts,
+          { min: 1, max: 50 }
+        );
+        break;
+      }
+      case 'security_lockout_duration_minutes': {
+        const minutes = normalizePositiveInteger(
+          'security_lockout_duration_minutes',
+          value,
+          DEFAULT_SECURITY_CONFIG.lockoutDurationMs / (60 * 1000),
+          { min: 1, max: 24 * 60 }
+        );
+        config.lockoutDurationMs = minutes * 60 * 1000;
+        break;
+      }
+      case 'security_attempt_window_minutes': {
+        const minutes = normalizePositiveInteger(
+          'security_attempt_window_minutes',
+          value,
+          DEFAULT_SECURITY_CONFIG.attemptWindowMs / (60 * 1000),
+          { min: 1, max: 24 * 60 }
+        );
+        config.attemptWindowMs = minutes * 60 * 1000;
+        break;
+      }
+      default:
+        break;
+    }
+  });
+
+  return config;
+}
+
+async function getSecurityConfig(options = {}) {
+  const now = Date.now();
+  const forceRefresh = options.forceRefresh === true;
+
+  if (!forceRefresh && cachedSecurityConfig && (now - cachedConfigFetchedAt) < SECURITY_CONFIG_CACHE_MS) {
+    return cachedSecurityConfig;
+  }
+
+  try {
+    const config = await loadSecurityConfigFromSettings();
+    cachedSecurityConfig = config;
+    cachedConfigFetchedAt = now;
+    return cachedSecurityConfig;
+  } catch (error) {
+    logger.error('Error loading security configuration:', error);
+    cachedSecurityConfig = { ...DEFAULT_SECURITY_CONFIG };
+    cachedConfigFetchedAt = now;
+    return cachedSecurityConfig;
+  }
+}
+
+function resetSecurityConfigCache() {
+  cachedSecurityConfig = { ...DEFAULT_SECURITY_CONFIG };
+  cachedConfigFetchedAt = 0;
+}
 
 /**
  * Track failed login attempt
@@ -59,6 +189,8 @@ async function trackSuccessfulLogin(identifier, ipAddress, userAgent) {
     if (!tableExists) {
       return;
     }
+
+    const { attemptWindowMs } = await getSecurityConfig();
     
     await db('login_attempts').insert({
       identifier,
@@ -69,7 +201,7 @@ async function trackSuccessfulLogin(identifier, ipAddress, userAgent) {
     });
 
     // Clear old failed attempts for this user
-    const cutoffTime = new Date(Date.now() - ATTEMPT_WINDOW);
+    const cutoffTime = new Date(Date.now() - attemptWindowMs);
     await db('login_attempts')
       .where('identifier', identifier)
       .where('success', formatBoolean(false))
@@ -83,30 +215,39 @@ async function trackSuccessfulLogin(identifier, ipAddress, userAgent) {
 /**
  * Check if account is locked due to too many failed attempts
  * @param {string} identifier - Username or email
+ * @param {string} [ipAddress] - Optional IP address scope
  * @returns {Promise<{isLocked: boolean, remainingTime?: number}>}
  */
-async function checkAccountLockout(identifier) {
+async function checkAccountLockout(identifier, ipAddress) {
   try {
     // Check if table exists first
     const tableExists = await db.schema.hasTable('login_attempts');
     if (!tableExists) {
       return { isLocked: false };
     }
+
+    const { attemptWindowMs, maxAttempts, lockoutDurationMs } = await getSecurityConfig();
     
-    const recentWindow = new Date(Date.now() - ATTEMPT_WINDOW);
+    const recentWindow = new Date(Date.now() - attemptWindowMs);
     
     // Get recent failed attempts
-    const failedAttempts = await db('login_attempts')
+    const failedAttemptsQuery = db('login_attempts')
       .where('identifier', identifier)
       .where('success', formatBoolean(false))
-      .where('attempt_time', '>=', recentWindow.toISOString())
-      .orderBy('attempt_time', 'desc')
-      .limit(MAX_LOGIN_ATTEMPTS);
+      .where('attempt_time', '>=', recentWindow.toISOString());
 
-    if (failedAttempts.length >= MAX_LOGIN_ATTEMPTS) {
+    if (ipAddress) {
+      failedAttemptsQuery.andWhere('ip_address', ipAddress);
+    }
+
+    const failedAttempts = await failedAttemptsQuery
+      .orderBy('attempt_time', 'desc')
+      .limit(maxAttempts);
+
+    if (failedAttempts.length >= maxAttempts) {
       // Check if still within lockout period
       const oldestAttempt = failedAttempts[failedAttempts.length - 1];
-      const lockoutEnd = new Date(oldestAttempt.attempt_time).getTime() + LOCKOUT_DURATION;
+      const lockoutEnd = new Date(oldestAttempt.attempt_time).getTime() + lockoutDurationMs;
       const now = Date.now();
 
       if (now < lockoutEnd) {
@@ -216,6 +357,6 @@ module.exports = {
   checkSuspiciousActivity,
   getGenericAuthError,
   initializeCleanupJob,
-  MAX_LOGIN_ATTEMPTS,
-  LOCKOUT_DURATION
-};
+  getSecurityConfig,
+  resetSecurityConfigCache
+};

backend/src/utils/requestIp.js

@@ -0,0 +1,36 @@
+/**
+ * Resolve the originating client IP address, accounting for reverse proxies.
+ * Returns the first entry from X-Forwarded-For when available, otherwise falls back
+ * to Express/Node connection properties.
+ * @param {import('express').Request} req
+ * @returns {string}
+ */
+function getClientIp(req) {
+  if (!req) {
+    return '';
+  }
+
+  const forwardedFor = req.headers['x-forwarded-for'];
+
+  if (typeof forwardedFor === 'string' && forwardedFor.length > 0) {
+    const [firstIp] = forwardedFor.split(',').map(part => part.trim()).filter(Boolean);
+    if (firstIp) {
+      return firstIp;
+    }
+  } else if (Array.isArray(forwardedFor) && forwardedFor.length > 0) {
+    const [firstIp] = forwardedFor;
+    if (firstIp) {
+      return firstIp.trim();
+    }
+  }
+
+  return (
+    req.ip ||
+    req.connection?.remoteAddress ||
+    req.socket?.remoteAddress ||
+    req.connection?.socket?.remoteAddress ||
+    ''
+  );
+}
+
+module.exports = { getClientIp };